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

Correctly uncheck previous default combination #20126

Merged
merged 1 commit into from Jul 13, 2020
Merged

Correctly uncheck previous default combination #20126

merged 1 commit into from Jul 13, 2020

Conversation

Amazzing
Copy link
Contributor

@Amazzing Amazzing commented Jul 9, 2020

Questions Answers
Branch? 1.7.7.x
Description? removeAttr('checked') doesn't actually uncheck the checkbox, so if you change default combination multiple times, then many inputs$('.attribute_default_checkbox') will remain checked, and only the last in DOM tree will be saved as default combination.
Type? bug fix
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? --
How to test? Open product sheet with 5-10 combinations. Most probably 1st combination is default. Set 5th combination as default and save the product. There will be Settings updated notification, and combination will be updated correctly.
After this don't reload page, and set 3rd combination as default and save product again. There will be Settings updated notification but default combination will not be updated correctly, you can check that after reloading page, or checking database table.

This change is Reviewable

@Amazzing Amazzing requested a review from a team as a code owner July 9, 2020 16:35
@prestonBot prestonBot added 1.7.7.x Branch Bug Type: Bug labels Jul 9, 2020
@Progi1984 Progi1984 added this to the 1.7.7.0 milestone Jul 10, 2020
@PierreRambaud PierreRambaud added the Waiting for QA Status: action required, waiting for test feedback label Jul 10, 2020
@khouloudbelguith
Copy link
Contributor

@khouloudbelguith khouloudbelguith added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Jul 13, 2020
@khouloudbelguith khouloudbelguith added this to To be merged in PrestaShop 1.7.7.3 Jul 13, 2020
@PierreRambaud PierreRambaud merged commit bc1feeb into PrestaShop:1.7.7.x Jul 13, 2020
@PierreRambaud
Copy link
Contributor

Thanks @Amazzing

@Progi1984 Progi1984 moved this from To be merged to Done in PrestaShop 1.7.7.3 Jul 15, 2020
@prestashop-issue-bot prestashop-issue-bot bot added the Fixed Resolution: issue closed because fixed label Jul 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.7.7.x Branch Bug Type: Bug Fixed Resolution: issue closed because fixed QA ✔️ Status: check done, code approved
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants