-
Notifications
You must be signed in to change notification settings - Fork 2.1k
[stable9] In cases where the server dictates the link share expiration the date… #25766
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[stable9] In cases where the server dictates the link share expiration the date… #25766
Conversation
|
@DeepDiver1975, thanks for your PR! By analyzing the annotation information on this pull request, we identified @PVince81, @blizzz and @rullzer to be potential reviewers |
| this.model.get('linkShare').expiration = expiration; | ||
| this.model.saveLinkShare({ | ||
| expiration: moment($target.val(), 'DD-MM-YYYY').format('YYYY-MM-DD') | ||
| expiration: expiration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I'd expect a successful ajax response to automatically trigger the model's update of the expiration.
But this is not true backbone so might need to do it manually in saveLinkShare after detecting success.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is necessary because the model itself does not change in between before and after the ajax call.
Let's say the server dictates an expiry to be 11.08.2016 and the user changes the date.
The response in the ajax will hold 11.08.2016 again and therefore not update the ui.
The user selected value will stay.
yes - a mess 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fine by me
|
any test steps ? |
|
Doesn't seem to work, the expiration date field doesn't update |
|
Works now 👍 |
|
Does this need a forward port to stable9.1 and master ? |
yes |
… is not updated on consequitive changes and enable/disable actions
643db0d to
aedae12
Compare
|
@DeepDiver1975 please forward port to stable9.1 and master |
|
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
… is not updated on consequitive changes and enable/disable actions