dqt: replace $.ajax with lorisFetch#10335
Conversation
|
Follow-up pushed to align with #9999 Updated
Notes
|
modules/dqt/jsx/react.app.js
Outdated
| const data = await this.client.getJSON( | ||
| loris.BaseURL + '/AjaxHelper.php?Module=dqt&script=' + script + '&' + | ||
| new URLSearchParams({ | ||
| category: rule.instrument, | ||
| field: rule.field, | ||
| value: rule.value, | ||
| }) | ||
| ); |
There was a problem hiding this comment.
the Client is designed to avoid manually contructed the URL like this, the Query class should be used in this case.
modules/dqt/jsx/DQTClient.js
Outdated
| getJSON(url) { | ||
| return this.fetchJSON(new URL(url, window.location.origin), { | ||
| method: 'GET', | ||
| headers: {'Accept': 'application/json'}, | ||
| }); | ||
| } |
There was a problem hiding this comment.
the get function in the Client class is designed to do this.
modules/dqt/jsx/react.app.js
Outdated
| const response = await lorisFetch( | ||
| `${loris.BaseURL}/dqt/dqt_setup/?format=json`, | ||
| {credentials: 'same-origin', method: 'GET'} | ||
| {method: 'GET'} |
There was a problem hiding this comment.
the Client get function with a Query param for the format and a setSubEndpoint should be able to handle this
modules/dqt/jsx/react.app.js
Outdated
| const response = await lorisFetch( | ||
| `${loris.BaseURL}/dqt/sessions/?format=json`, | ||
| {credentials: 'same-origin', method: 'GET'} | ||
| {method: 'GET'} |
There was a problem hiding this comment.
the Client get function with a Query param for the format and a setSubEndpoint should be able to handle this
modules/dqt/jsx/DQTClient.js
Outdated
| }); | ||
| if (!response.ok) { | ||
| const error = new Error('request_failed'); | ||
| error.status = response.status; | ||
| error.response = response; | ||
| throw error; | ||
| } | ||
| return response.text(); | ||
| } | ||
| } |
There was a problem hiding this comment.
I don't think we should use fetch or fetchJSON directly here. The Client class is an abstraction layer designed to handle URL building and error reporting for you. By calling fetch directly, you're losing the /dqt base path and our custom error-handling system.
If you need to fetch raw text or send URL-encoded data, I would do that using lorisFetch.
modules/dqt/jsx/react.app.js
Outdated
| curRequest = this.client.getJSON( | ||
| loris.BaseURL | ||
| + '/AjaxHelper.php?Module=dqt&script=GetDoc.php&DocID=' | ||
| + encodeURIComponent(this.state.queryIDs[key][i])), { | ||
| data: { | ||
| DocID: this.state.queryIDs[key][i], | ||
| }, | ||
| dataType: 'json', | ||
| }).then((value) => { | ||
| let queries = this.state.savedQueries; | ||
| queries[value._id] = value; | ||
| this.setState({savedQueries: queries}); | ||
| }); | ||
| + encodeURIComponent(this.state.queryIDs[key][i]) | ||
| ) |
There was a problem hiding this comment.
the Client get function with a Query param for the format and a setSubEndpoint should be able to handle this
modules/dqt/jsx/react.app.js
Outdated
| this.client.getJSON( | ||
| loris.BaseURL + '/dqt/ajax/datadictionary.php?' + | ||
| new URLSearchParams({keys: JSON.stringify(fieldsList)}) | ||
| ) | ||
| .then((data) => { |
There was a problem hiding this comment.
the Client get function with a Query param for the format and a setSubEndpoint should be able to handle this
There was a problem hiding this comment.
Across the refactors I notice a pattern here of bypassing the established Http.Client abstraction. By using native fetch, manual URL string concatenation, and custom error handling inside the Client extension classes, we lose the benefits of our centralized system for predictable URL building, standardized logging, error and response handling.
Our design goal is to keep the Client strictly for modern JSON/REST interactions. As I outlined in my original comment on your lorisFetch PR, I think legacy PHP helpers that require URL-encoding or return raw text should use your lorisFetch.
HenriRabalais
left a comment
There was a problem hiding this comment.
Client implementation looks good!
Summary
Replace
$.ajaxcalls withlorisFetchindqt.Why
This removes jQuery AJAX usage in this module for issue #4213.
Scope
Only AJAX replacement in this module. No unrelated jQuery refactors.
Dependency
Depends on PR #10333.
Verification
git grep '\$\.ajax'is clean for touched files.