Update keepalive() to use new /api/{command} URL format#910
Update keepalive() to use new /api/{command} URL format#910
Conversation
The keepalive() method in Network.jsx was always using addCommandToUrl() which produces the legacy /api/?&command=X format, even when the endpoint is configured for the new URL format. This mirrors the existing isNewURLFormat handling already present in the post() method. Co-authored-by: Nathan Metcalf <nathanmetcalf@users.noreply.github.com>
|
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
| } else { | ||
| // Add the API command to our URL (for console debugging purposes) | ||
| url = addCommandToUrl(parameters.command, url); | ||
| } |
There was a problem hiding this comment.
NAB - we don't need to keep the old-style format at this point. We could remove this whole else block and the if (isNewURLFormat)
|
Testing now... |
| if (isNewURLFormat) { | ||
| // Remove command from parameters and use it in the URL | ||
| const command = parameters.command; | ||
| // eslint-disable-next-line no-param-reassign | ||
| delete parameters.command; | ||
| url = `${endpoint}${command}`; | ||
| } else { | ||
| // Add the API command to our URL (for console debugging purposes) | ||
| url = addCommandToUrl(parameters.command, url); |
There was a problem hiding this comment.
Wait, that's the wrong thing to update. @MelvinBot we need to update the function addCommandToUrl instead. It should not use ${newUrl}&command=${command}, but instead of:
- Ensure that the passed
urlends with/, or append/ - Return append the
commandafter that, i.e. return something like${url}/${command}
There was a problem hiding this comment.
@MelvinBot Read the above and do the needful.
We also want to pass the CODE_REVISION as a new param called appVersion. We need to check if clients are using the newest revision. Do that too.
|
Wait, that's the wrong thing to update. @MelvinBot we need to update the function addCommandToUrl instead. It should not use ${newUrl}&command=${command}, but instead of: |
|
I don't think Melvin listened |
|
Wait, that's the wrong thing to update. @MelvinBot we need to update the function addCommandToUrl instead. It should not use ${newUrl}&command=${command}, but instead of: Ensure that the passed url ends with /, or append / |
The
keepalive()method in Network.jsx always usedaddCommandToUrl()to construct the legacy/api/?&command=XURL format, even when the endpoint is configured for the new/api/{command}format (trailing slash). This updateskeepalive()to checkisNewURLFormatand build the URL path correctly, mirroring the logic already present in thepost()method. The command is also removed from the parameters object so it isn't sent in the FormData body.Fixed Issues
$ https://github.com/Expensify/Expensify/issues/625277
Tests
// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review"
// Please describe what tests you performed that validates your changed worked.
QA
// TODO: The human co-author must fill out the QA tests you ran before marking this PR as "ready for review".
// Please describe what QA needs to do to validate your changes and what areas do they need to test for regressions.