Skip to content
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

Bugfix: Prevent wrong dirty reset #4761

Merged
merged 2 commits into from
Mar 17, 2024

Conversation

manuelmeister
Copy link
Member

@manuelmeister manuelmeister commented Mar 13, 2024

Initially, we reset the dirty flag before each save request.
My PR defines dirty as "local value is not the same as api value", on devel it is currently defined as "time between entering a different value and starting to patch to api".

Some api fields have a InputFilter/Trim in the backend, which means that the api value returned will never contain spaces around the value. If the user leaves the input, the localValue should be reset to the apiValue, to correctly represent the actual db value, without disrupting the user.

Closes #3875

@manuelmeister manuelmeister added Bug deploy! Creates a feature branch deployment for this PR External feedback This Issue is Based on Feedback from the Community labels Mar 13, 2024
@manuelmeister manuelmeister marked this pull request as ready for review March 13, 2024 07:41
Copy link

github-actions bot commented Mar 13, 2024

Feature branch deployment currently inactive.

If the PR is still open, you can add the deploy! label to this PR to trigger a feature branch deployment.

Copy link
Member

@carlobeltrame carlobeltrame left a comment

Choose a reason for hiding this comment

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

Perfect fix, thank you!
I think this also fixes #4404, is that possible? I wasn't able to reproduce it anymore, even with extremely throttled network connection

@manuelmeister
Copy link
Member Author

@carlobeltrame I don't know, that would be totally awesome of course 😂

@manuelmeister
Copy link
Member Author

@usu can you also check this, to make sure I did not make any mistakes or errors of thinking.

@manuelmeister manuelmeister mentioned this pull request Mar 17, 2024
@manuelmeister manuelmeister added this pull request to the merge queue Mar 17, 2024
Merged via the queue into ecamp:devel with commit e08a8f1 Mar 17, 2024
32 checks passed
@manuelmeister manuelmeister deleted the bugfix/api-wrapper-dirty branch March 17, 2024 16:56
@usu
Copy link
Member

usu commented Mar 24, 2024

@usu can you also check this, to make sure I did not make any mistakes or errors of thinking.

Sorry, didn't manage to test this before it got merged. It seems that on blur, the dirty flag is not properly reset (see screen recording below).

Screen.Recording.2024-03-24.at.11.03.44.mov

My PR defines dirty as "local value is not the same as api value", on devel it is currently defined as "time between entering a different value and starting to patch to api".

The 2nd seems like a bug to me, but the first one is also problematic. I labeled #2578 with "Meeting discuss" we we can have a look at it during next meeting.

@carlobeltrame carlobeltrame added the Meeting Discuss Am nächsten Core-Meeting besprechen label Mar 26, 2024
@carlobeltrame carlobeltrame removed the Meeting Discuss Am nächsten Core-Meeting besprechen label Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug deploy! Creates a feature branch deployment for this PR External feedback This Issue is Based on Feedback from the Community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adjust trim on autosave to keep 1 space char
4 participants