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

N°7371 - Remembers the choice done in the popup that appears when saving public and private logs at the same time #636

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

accognet
Copy link
Contributor

Internal

@accognet accognet added the bug Something isn't working label Mar 20, 2024
@accognet accognet self-assigned this Mar 20, 2024
@Molkobain Molkobain added the internal Work made by Combodo label Mar 20, 2024
@Molkobain
Copy link
Contributor

Please set a PR title explaining the symptom; not a generic one.

@Hipska
Copy link
Contributor

Hipska commented Mar 20, 2024

Please set a PR title explaining the symptom; not a generic one.

Yes, as it now looks like this PR introduces that bug..

@accognet accognet changed the title N°7371 - Bug when saving public and private log at same time N°7371 - Remembers the choice done in the popup that appears when saving public and private logs at the same time Jul 30, 2024
Comment on lines +218 to +222
{operation: 'set_pref', code: sPreferenceCode, value: sPrefValue}, function (data) {
//do nothing
}).done(function() {
oUserPreferences[sPreferenceCode] = sPrefValue;
}); // Make it persistent
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment is misleading. Making the POST call actually persist the preference, so it doesn't do "nothing" and it's not L221 that persists it (it updates local state).

Comment on lines 476 to 478
if ((GetUserPreference('activity_panel.show_multiple_entries_submit_confirmation',true) === true
|| GetUserPreference('activity_panel.show_multiple_entries_submit_confirmation', true) === "true")
&& (Object.keys(this._GetEntriesFromAllForms()).length > 1)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels messy, if the return type isn't strict maybe you should wrap this in a helper to always return a boolean.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to find what the "real" value is. I did a lot of testing and changed the value to 'true' many times that's why this test doesn't look good.

@accognet accognet force-pushed the feature/7371-Alert_saving_public_and_private_log branch from 85a8e96 to a47469e Compare December 10, 2024 08:54
@Molkobain
Copy link
Contributor

image

Careful, rebase of the branch wasn't done correctly, you have 237 commits coming to support/3.2 ! :)

@accognet accognet changed the base branch from support/3.2 to develop December 10, 2024 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working internal Work made by Combodo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants