Skip to content

[fix] Unsaved changes js trigger when device have location defined #388 #405

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

Closed

Conversation

jalajcodes
Copy link

Unsaved changes js is triggered when device have geographic location defined,
fixed it by ignoring the input field for device location inside unsaved_changes.js

Fixes #388

unsaved changes js is triggered when device have geographic location defined,
fixed it by ignoring the input field for device location

Fixes openwisp#388
@jalajcodes jalajcodes changed the title [fix] Unsaved changes js trigger when device has location defined #388 [fix] Unsaved changes js trigger when device have location defined #388 Mar 13, 2021
@coveralls
Copy link

Coverage Status

Coverage remained the same at 99.085% when pulling f0d38ef on jalajcodes:issues/388-unsaved-changes-js into 460c86c on openwisp:master.

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jalajcodes, see my comment below.

name.indexOf('root') === 0) {
name.indexOf('root') === 0 ||
// ignore device location field
name.substr(0, 14) == 'devicelocation') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this partially solves the issue, but what if the user changes the device location and forgets to save?
Can you find a way to make that case work as well?

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jalajcodes any update on this? If you can't finish it let me know, I need to get this done soonish because it's a very annoying bug for our users.

@jalajcodes
Copy link
Author

jalajcodes commented Mar 24, 2021

@jalajcodes any update on this? If you can't finish it let me know, I need to get this done soonish because it's a very annoying bug for our users.

For some reason, I am now unable to reproduce the issue, I deleted the repo and started fresh but still unable to reproduce it

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jalajcodes any update on this? If you can't finish it let me know, I need to get this done soonish because it's a very annoying bug for our users.

For some reason, I am now unable to reproduce the issue, I deleted the repo and started fresh but still unable to reproduce it

Try creating two locations in the system.

Then create a device, assign it an existing location, save and reopen the same page.

Now change the location of the device but do not save, just click any other link to exit the device page, we expect to get the unsaved changes alert in this case, but it's not triggering because the code you provided excluded the devicelocation attribute from the check, while instead we have to understand why when the devicelocation is present it is interpreted as always different by the JS code.

@nemesifier
Copy link
Member

Superseded by #411.
Thanks for trying @jalajcodes 👍

@nemesifier nemesifier closed this Mar 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

[bug] Unsaved changes JS broken when device have geographic location defined
3 participants