fixed 404 issue when pressing cancel #175
Conversation
|
@Ducica thanks a lot for this fix, I tested it and it works. Not related to your changes, but the fact that we stay in the same page is not super user-friendly... We will add the version bump in the a dedicated release commit. |
invenio_userprofiles/__init__.py
Outdated
| @@ -31,7 +31,7 @@ | |||
| from .ext import InvenioUserProfiles | |||
| from .models import UserProfile, UserProfileProxy | |||
|
|
|||
| __version__ = "4.0.0" | |||
| __version__ = "4.0.1" | |||
There was a problem hiding this comment.
Can you please remove this change?
There was a problem hiding this comment.
@ntarocco Hey, thank you both for the quick reaction. I've made the changes as requested. Out of curiosity, what would you say the desirable behavior would be? I understood the goal is to just flush any changes that user was making, so this solution gets the job done. Of course full page refresh is not ideal, but it is a MPA anyway.
There was a problem hiding this comment.
@ntarocco Hey, thank you both for the quick reaction. I've made the changes as requested. Out of curiosity, what would you say the desirable behavior would be? I understood the goal is to just flush any changes that user was making, so this solution gets the job done. Of course full page refresh is not ideal, but it is a MPA anyway.
The behaviour is not consistent. In other places, when we hit the Cancel button, we leave the page and go back/somewhere else. Anyway, not a big deal.
Thanks a lot for the fix!
There was a problem hiding this comment.
@ntarocco thank you for the merge!
Ducica
force-pushed
the
issue-171-Cancel-button-in-profile-settinsg-leads-to-404
branch
from
b4a68df to
f60b932
Compare
CHANGES.rst
Outdated
| @@ -9,6 +9,10 @@ | |||
| Changes | |||
| ======= | |||
|
|
|||
| Version 4.0.1 | |||
There was a problem hiding this comment.
this is good, but should go with the version change in init into a separate commit which then contains the addition in the CHANGES.rst file and the version change in the invenio_userprofiles/__init__.py file
There was a problem hiding this comment.
@utnapischtim Sorry, my bad. So just to be clear, final result shall be one commit for actual changes and then 1 commit where I write to changes.rst and init.py? Do I need to place some date into changes.rst (released 2025-01.07) or is this not necessary? Please let me know.
There was a problem hiding this comment.
yes, it should be one commit for the actual changes, or more if the changes should be committed by more than one commit. more info here and one commit for the version changes which includes the addition to CHANGES.rst and __init__.py
the addition in CHANGES.rst should follow how the other release notes have been written like
Version 4.0.1 (released 2025-01-07)
- title of commit message
but normally you don't have to add this commit to your PR, because it could be that the changes are not released directly afterwards, or because there are other commits in the history and the changes.rst has to be updated anyway. or the chosen version is wrong.
better only add your changes to the PR and let the maintainer do the work on releasing your changes.
There was a problem hiding this comment.
@utnapischtim Thanks, I have left just the changes to the code itself as suggested in your previous comment. Regards, Dusan
Ducica
force-pushed
the
issue-171-Cancel-button-in-profile-settinsg-leads-to-404
branch
from
f60b932 to
160e9c1
Compare
Description
Resolution to issue #171
It is probably not the safest to use href="." when goal is to reload the current page. I used url_for, another option would be something like href="javascript:window.location.reload(true)", but feel that url_for, might be the safer solution.