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

Fixed detect changes in custom options section (product edit) #2444

Merged
merged 7 commits into from
Sep 26, 2022
Merged

Fixed detect changes in custom options section (product edit) #2444

merged 7 commits into from
Sep 26, 2022

Conversation

fballiano
Copy link
Contributor

@fballiano fballiano commented Aug 17, 2022

This PR should fix #1693, when editing custom options for a product (in the backend) changes (the floppy disk icon appearing in the left sidebar) are not always detected correctly.

In app/design/adminhtml/default/default/template/catalog/product/edit/options/option.phtml I moved this line:

productOption.bindRemoveButtons();

a little later in the code, so that it gets executed when the custom option's options are already rendered.

plus I added some more "check if element is changed" to the app/design/adminhtml/default/default/template/catalog/product/edit/options/type/select.phtml file

@github-actions github-actions bot added Component: Adminhtml Relates to Mage_Adminhtml Component: Catalog Relates to Mage_Catalog Template : admin Relates to admin template labels Aug 17, 2022
@addison74
Copy link
Contributor

I tested all the options in the "Custom Options" section.

Title / Input Type / Is Required / Sort Order => OK
Title / Price / Price Type / SKY / Sort Order => OK
[X] delete a row action => OK
[Add New Row] action => OK

Observation
[Delete Option] action is not working in all tabs. Should it detect this change?

@fballiano
Copy link
Contributor Author

Observation [Delete Option] action is not working in all tabs. Should it detect this change?

I feel like it was working to me (before this PR), just as a double check, when you hit the "delete option" the "floppy" icon doesn't show right?
What do you mean by "in all tabs"?

@addison74
Copy link
Contributor

I feel like it was working to me (before this PR), just as a double check, when you hit the "delete option" the "floppy" icon doesn't show right?

This is BEFORE pressing the [x Delete Option] button.

before

This is AFTER pressing the [x Delete Option] button.

after

In my test (OM -19 + your PR) it did not detect the action and the save icon warning is missing.

What do you mean by "in all tabs"?

In sections on the left side menu in product edit page. For Price section I already posted an issue here #2448.

@fballiano
Copy link
Contributor Author

I'll recheck ASAP, thanks!

@fballiano
Copy link
Contributor Author

should be fixed now!

@github-actions github-actions bot added the Component: Bundle Relates to Mage_Bundle label Aug 18, 2022
@fballiano
Copy link
Contributor Author

I just committed a fix for #2449

@fballiano
Copy link
Contributor Author

I should have fixed #2450 too and a bug that actually didn't allow a downloadable sample to be removed.

@addison74
Copy link
Contributor

addison74 commented Aug 18, 2022

In Custom Option tab by clicking on the [x Delete Option] button OM detects the change, the save icon warning appears => OK

In Bundle Items tab all changes reported here #2449 are now detected, the save icon warning appears this time => OK

In Downloadable Information tab all changes reported here #2450 are now detected, the save icon warning appears too => OK

You may fix this one #2448 too to close another open issue.

@github-actions github-actions bot added the Component: Downloadable Relates to Mage_Downloadable label Aug 18, 2022
@fballiano
Copy link
Contributor Author

Also #2448 should be fixed now :-) :-)

@addison74
Copy link
Contributor

addison74 commented Aug 19, 2022

In Prices tab all changes reported here #2448 are now detected, the save icon warning appears too => OK

addison74
addison74 previously approved these changes Aug 19, 2022
@fballiano
Copy link
Contributor Author

ok also #2454 should be fixed

@OpenMage OpenMage deleted a comment from fballiano Aug 20, 2022
@addison74
Copy link
Contributor

In Associated Products tab for a configurable product, all changes reported here #2454 are now detected, the save icon warning appears too => OK

Considering that I have not identified any other actions in the editing of the product that are not detected, the PR can be approved and merged. Thank you for your work @fballiano.

addison74
addison74 previously approved these changes Aug 20, 2022
@fballiano fballiano requested a review from kiatng August 31, 2022 12:26
@fballiano
Copy link
Contributor Author

@kiatng @elidrissidev @sreichel sorry to bother, could somebody check this PR? 🙏

@sreichel
Copy link
Contributor

I "love" frontend tests ;) (Someone to implement selenium?)

Did some tests and looks good so far. Is it "possible" to detect changes, that have been reverted to original value.

@fballiano
Copy link
Contributor Author

Is it "possible" to detect changes, that have been reverted to original value.

at the moment there's nothing (in the backend) that does such a think so i'd leave it for other PRs

@fballiano fballiano merged commit 4a952b0 into OpenMage:1.9.4.x Sep 26, 2022
@fballiano fballiano deleted the issue_1693 branch September 26, 2022 14:52
@github-actions
Copy link
Contributor

Unit Test Results

1 files  1 suites   0s ⏱️
0 tests 0 ✔️ 0 💤 0 ❌
7 runs  5 ✔️ 2 💤 0 ❌

Results for commit 4a952b0.

@addison74
Copy link
Contributor

Is it "possible" to detect changes, that have been reverted to original value.

This option can be useful only when you make changes that you can retain and restore to the initial value. For example, you changed the price from 100 to 120, the save warning icon appears, but you change it back to the value of 100. It would be natural for the icon to disappear.

If you make several changes in the sections and you don't remember them, then the only option to return to the initial state is to press the [Reset] button. I don't know what this modification entails, but it is a complex one. Just think if you delete a row from a custom option, it is not possible to return to the previous state, as in the case of text or select fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Adminhtml Relates to Mage_Adminhtml Component: Bundle Relates to Mage_Bundle Component: Catalog Relates to Mage_Catalog Component: Downloadable Relates to Mage_Downloadable JavaScript Relates to js/* Template : admin Relates to admin template
Projects
None yet
4 participants