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

[shopsys] entities are refreshed after product visibility recalculations #2202

Merged
merged 3 commits into from
Feb 2, 2021

Conversation

grossmannmartin
Copy link
Member

Q A
Description, reason for the PR entities are, by default, not refreshed in identity map after update with native query. This PR add a manual refresh of loaded entities, so it's safe to work with those entites after recalculations (=native queries).
New feature No
BC breaks No
Fixes issues closes #2108
Have you read and signed our License Agreement for contributions? Yes

@grossmannmartin grossmannmartin force-pushed the mg-refresh-entity-after-recalculations branch 2 times, most recently from bf5324f to 91fe882 Compare January 25, 2021 14:58
@malyMiso
Copy link
Contributor

malyMiso commented Jan 26, 2021

Hi,

I was asked to check if this PR resolves scenario described in #2108. I have a question about performance. Couldn't it have an impact on performance while processing large amount of data, e.g. ProductVisibilityMidnightCronModule, or custom cron modules e.g importing data from IS.

However, I like this solution with provided test and it solves described problem. 👍

@TomasLudvik TomasLudvik added the Bug Something isn't working label Jan 27, 2021
@TomasLudvik TomasLudvik added this to the SSFW 9.1.1 milestone Jan 27, 2021
@grossmannmartin
Copy link
Member Author

Hi,

I was asked to check if this PR resolves scenario described in #2108. I have a question about performance. Couldn't it have an impact on performance while processing large amount of data, e.g. ProductVisibilityMidnightCronModule, or custom cron modules e.g importing data from IS.

However, I like this solution with provided test and it solves described problem. 👍

Thank you for looking at this!
It's true that this can have a negative impact on the performance – most notably on the database (new queries are performed).

I'm afraid it's the price for updated data.

@grossmannmartin grossmannmartin force-pushed the mg-refresh-entity-after-recalculations branch from 91fe882 to 811f274 Compare January 31, 2021 21:42
@malyMiso
Copy link
Contributor

malyMiso commented Feb 2, 2021

Hi,
I was asked to check if this PR resolves scenario described in #2108. I have a question about performance. Couldn't it have an impact on performance while processing large amount of data, e.g. ProductVisibilityMidnightCronModule, or custom cron modules e.g importing data from IS.
However, I like this solution with provided test and it solves described problem. +1

Thank you for looking at this!
It's true that this can have a negative impact on the performance – most notably on the database (new queries are performed).

I'm afraid it's the price for updated data.

I understand. I'm thinking about giving an option to disable refreshing entities on purpose. Adding parameter to refreshProductsVisibility could solve it (or new function ?). It would always refresh entities by default and PRG can change it explicitly

In cron modules, entities won't be refreshed
In a request, entities will be refreshed

Does it make sense ?

@TomasLudvik TomasLudvik force-pushed the mg-refresh-entity-after-recalculations branch from 811f274 to 8655282 Compare February 2, 2021 12:25
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 2, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@TomasLudvik TomasLudvik merged commit b8773f5 into 9.1 Feb 2, 2021
@TomasLudvik TomasLudvik deleted the mg-refresh-entity-after-recalculations branch February 2, 2021 12:43
@TomasLudvik TomasLudvik changed the title [shopsys] entites are refreshed after product visibility recalculations [shopsys] entities are refreshed after product visibility recalculations Feb 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants