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

set changed status on model to prevent status overwriting when model gets saved #580

Closed
wants to merge 2 commits into from

Conversation

airbone42
Copy link

In Cron/Model/Observer the tryLockJob method gets called on the model but the same model is later saved in the main method call. Unfortunately in the memory of the model is still the old status stored so it gets then overwritten and the job is no longer locked and gets triggered multiple times.

This is also a Magento 1 problem, I'm looking forward to a backport, as we have to fix this in every single instance. :(

@airbone42
Copy link
Author

OK I've seen that you added some code in the observer to deal with that in one of the latest releases. unfortunately that's still not there in mage1 and imho the schedule model should deal with that, not the observer, to also make it reusable.

@verklov
Copy link
Contributor

verklov commented Jun 2, 2014

@airbone42, so the issue is no more relevant for Magento 2, right? For Magento 1, there is no GitHub repository unfortunately. Piotr Kaminski is the main point of contact for Magento 1. Please contact @piotrekkaminski on Twitter. He will be able to respond and take your contribution into account in the Magento 1 backlog. Thank you for your contribution!

P.S. Since it appears the team has resolved the issue to be fixed by your code contribution, could you please close this pull request once you get in contact with Piotr? Thank you!

@airbone42
Copy link
Author

Hi @verklov,

when you check the code you can see that it was technically fixed but just with a workaround. The real solution should be as shown in the pull request. The Schedule model itself should deal with that, why should the observer fix an unexpected behavior for the schedule model?

Regards,
Tobias.

@verklov
Copy link
Contributor

verklov commented Jun 3, 2014

OK, got you. So I will create a ticket in regards to Magento 2 for the team to evaluate the look at the solution you suggest. Thank you!

@verklov verklov assigned verklov and unassigned piotrekkaminski Jun 3, 2014
@airbone42
Copy link
Author

Is there any update on this?

magento-team added a commit that referenced this pull request Jul 11, 2014
* Service layer updates:
  * Created Category service and methods
  * Renamed attribute option service
  * Implemented an API method to remove for attribute options
  * Created TaxClass service and methods
  * Created APIs for Tax service
* Framework improvements:
  * REST/SOAP calls uses default store if store code not provided
  * Added a warning about using a not secure protocol for theidentity link URL
  * Fixed exception masking and removed unnecessary exceptions from the Webapi framework
* WEEE features parity:
  * Fixed an issue with Tax calculations when FPT is enabled
  * Fixed an issue where FPT was not included in the subtotal number on invoice pages
  * Fixed an issue where FPT was not included in the subtotal number on credit memo pages
  * Free shipping calculated with FPT
  * Fixed an issue where discounts where applied to FPT
  * Fixed an issue with rounding is the Tax detailed info
  * Fixed issues with bundle product pricing with tier and special prices
* Added an integrity test to verify that dictionary and code are synced
* i18n Improvements:
  * Improved the wording of the i18n CLI Tools
  * Removed the helpers which became unused after i18n Improvements
* Fixed bugs:
  * Fixed an issue where configurable attributes were not chosen according to the hash tag
  * Fixed an issue where the Compare Products functionality did not work correctly
  * Fixed an issue where product attribute values were duplicated after import
  * Fixed an issue were the scope of an attribute was not considered in catalog price rule conditions
  * Fixed an issue where shipping address was not saved if it was added during checkout
  * Fixed an issue where there was no POST request when saving a customer group
  * Fixed an issue where an attribute template was not applied after changing it for the first time during product creation
  * Fixed an issue where the Sale Report Grid with no results found contained an unnecessary empty Total section
  * Fixed an issue where a notice was added to system.log when a product was added to cart
  * Fixed integration test coverage failure
  * Fixed an issue where a message about inequality of password and confirmation was displayed in the wrong place
  * Fixed an issue with an XSS warning in 'Used for Sorting in Product Listing' property of Product Attribute
  * Fixed an issue where an order was not displayed  on frontend if its order status was deleted
  * Fixed an issue where  tier pricing was not displayed on a grouped product page
  * Verified and fixed the content of errors returned from SOAP calls
  * Fixed an issue where it was impossible to create a tax rule when using a complex Customer/Product tax class
  * Fixed an issue where the Street Address line count setting was not applied.
  * Fixed an issue where customers were not assigned to the correct VAT customer groups during admin order creation
  * The unused translateArray method of AbstractHelper was removed
  * Fixed an issue where localization did not work for strings containing a single quote (')
  * Fixed issues with  the translate and the logging transformation tools
  * Fixed an issue where it was impossible to create a URL rewrite for a CMS Page with Temporary (302) or Permanent (301) redirect
* GitHub requests:
  * [#598] Add Sort Order to Rules
  * [#580] Set changed status on model to prevent status overwriting when model gets saved
* Unit Tests Coverage:
  * Part of the Catalog module covered with the unit tests
* Added the following functional tests:
  * Applying Several Catalog Price Rules
  * Attribute Set Creation
  * Category Deletion
  * Customer Group Deletion
  * Generating Sitemap
  * Product Attribute Deletion
  * Update Admin User
  * Update Cms Page
  * Update Customer Group
  * Update Downloadable Product
  * Update Product Attribute
  * Update Sales Rule
  * Update Sitemap
@verklov
Copy link
Contributor

verklov commented Jul 21, 2014

@airbone42, the team processed your contribution. The fix was released in dev86. Thank you for your contribution and looking forward to your new bug reports and pull requests. We are closing this issue.

@verklov verklov closed this Jul 21, 2014
sivaschenko added a commit that referenced this pull request Sep 9, 2015
magento-team pushed a commit that referenced this pull request May 6, 2016
okorshenko pushed a commit that referenced this pull request Dec 14, 2016
magento-devops-reposync-svc pushed a commit that referenced this pull request Dec 4, 2023
ACPT-1697: Move state-*-list.php files back to dev from lib
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.

3 participants