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

Fix#8925: unable to set back a value to the inherited default value on profile #8926

Merged
merged 2 commits into from
Sep 4, 2023

Conversation

Clem-Fern
Copy link
Contributor

@Clem-Fern Clem-Fern commented Sep 4, 2023

Hey @Eugeny,

As discuss in #8912 and describe in #8925, my previous PR #8726 seems to have bring a new bug regarding the Disable dynamic tab title and Clear terminal after connection options (really sorry about that :/). At the time of the PR, I only notice this problem with the group attribute. I think we do not have other choice than deleting profile fields one by one in writeProfile method to prevent having ghost attributes in the profile config.

Without deleting the field one by one, Object.assign call does not delete undefinied value from the profile object (Context from #8726):

What's the reason for writeProfile deleting the fields one by one? Is it so that default values get removed? That should already work automatically via ConfigProxy.

When a profile is updated, the group attribute is set to undefinied if no group has been selected. Because of that, Object.assign do not seem to override the group attribute and leave the old value. It made impossible to reassign a profile in Ungrouped. But, now at head rested, I think this behavior only happens with the group attribute. It would probably be easier to change a bit the EditProfileModal to leave an empty string in the group attribute when no group has been selected.

Anyway, as we now provide profile object copy with getProfileGroups, profile field suppression can only be done inside writeProfile method. (related refactoring in e4169a5)

I tried my best to describe what I found, I hope all of this is clear enough ^^'
Feel free to ask if any changes needed

Resolve #8925

@Eugeny Eugeny merged commit 3ce2bb6 into Eugeny:master Sep 4, 2023
10 checks passed
@Eugeny
Copy link
Owner

Eugeny commented Sep 4, 2023

Makes sense and thank you for the fix!

@Clem-Fern Clem-Fern deleted the ref-profiles-v2 branch November 19, 2023 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Profile Settings: unable to set back an option to the inherited default value on profile
2 participants