chore: move xhr management to a module and refactor #1555
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yohanboniface
force-pushed
the
request-module
branch
from
9a7d86d to
7409ae5
Compare
almet
reviewed
Jan 29, 2024
almet
reviewed
Jan 29, 2024
yohanboniface
force-pushed
the
request-module
branch
3 times, most recently
from
13b2703 to
0e6b73c
Compare
yohanboniface
changed the title
davidbgk
reviewed
Jan 31, 2024
davidbgk
reviewed
Jan 31, 2024
almet
reviewed
Feb 1, 2024
almet
reviewed
Feb 1, 2024
yohanboniface
force-pushed
the
request-module
branch
from
99438ec to
a47cd2f
Compare
It sort of work for common cases, but edge cases needs work, specifically the login flow.
Instead of dealing with in JavaScript, let's do a more classic HTTP flow. The main flows work, but there is still at least one to deal with: when editing a map without being logged in, the server may ask for login, and in this case we should login THEN reissue the request, so we need to interrupt the first request in some way, otherwise the server will still answer with a 403, which is what happens after this commit.
I decided to remove the check `is_ajax` from `validate_url` to simplify and edge case, and because I think it was more or less useless. Basically, when getting remote data, we have two cases: - direct call to the remote URL - proxy through our `ajax_proxy` system (to work around CORS limitations) In the first case, we cannot set the `X-Requested-With` header, otherwise preflight step will fail, and in the second case, until now, we needed to set this header for this `is_ajax` check to pass. So keeping this check would mean adapting the behaviour of the Request/ServerRequest class in a non elegant way. So let's make it simple…
And use UI for dealing with dataloading/dataload events (used for progress bar).
yohanboniface
force-pushed
the
request-module
branch
from
a47cd2f to
0ebb761
Compare
davidbgk
approved these changes
Feb 5, 2024
|
💣 |
davidbgk
added a commit
that referenced
this pull request
Apr 5, 2024
When the `UMAP_ALLOW_ANONYMOUS` setting is False, we return the login url through a JSON response. We lost that ability in #1555. The JS part was not following that link in that particular case and lead to more errors because the map was not saved (hence, no `map_id`). For now, the current work on the map is lost because of the redirection and we have a confirmation dialog to quit the edited page with unsaved changes. Maybe we should display a custom message instead of a brutal redirection? Like: you’re not logged in, do it in a separate tab to keep you work? (A bit ugly…) Sidenote: we might want to use the `redirect` pattern/key in the JSON response that we already use for deletion and clone for consistency.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It sort of work for common cases, but edge cases needs work, specifically the login flow.