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.

Sign up for GitHub

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

Jump to bottom

[10.0][FIX] Fix shopinvader - Variants/Product active/inactive #568

Merged
merged 1 commit into from
Feb 13, 2020
Merged

[10.0][FIX] Fix shopinvader - Variants/Product active/inactive #568

merged 1 commit into from
Feb 13, 2020

Conversation

acsonefho
Copy link
Contributor

Currently when every shopinvader variants related to a shopinvader product are disabled, this shop. product is also disabled (active = False).

But when at least 1 shopinvader variant is re-enabled, we should also re-enable the related shop. product (active = True).
Otherwise it could cause some trouble into url management (url.url) who could stay disabled (side effect).

@codecov-io
Copy link

codecov-io commented Feb 4, 2020 •
edited
Loading

Codecov Report

Merging #568 into 10.0 will increase coverage by 0.13%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##             10.0     #568      +/-   ##
==========================================
+ Coverage   89.61%   89.74%   +0.13%     
==========================================
  Files         142      143       +1     
  Lines        3735     3773      +38     
==========================================
+ Hits         3347     3386      +39     
+ Misses        388      387       -1
Impacted Files Coverage Δ
shopinvader/models/shopinvader_variant.py 98.24% <100%> (-0.82%) ⬇️
shopinvader_promotion_rule/models/sale_order.py 100% <0%> (ø)
shopinvader/models/sale.py 95.29% <0%> (+1.17%) ⬆️
shopinvader/services/cart.py 89.14% <0%> (+1.2%) ⬆️
shopinvader/services/abstract_sale.py 100% <0%> (+2.43%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4ddd44c...70609f3. Read the comment docs.

simahawk
simahawk requested changes Feb 4, 2020
View reviewed changes
shopinvader/models/shopinvader_variant.py Outdated Show resolved Hide resolved
shopinvader/models/shopinvader_variant.py Outdated Show resolved Hide resolved
@simahawk simahawk added the 10.0 label Feb 4, 2020
@simahawk simahawk added this to the 10.0 milestone Feb 4, 2020
@acsonefho acsonefho force-pushed the 10.0-shopinvader_variant_re_enabled branch 2 times, most recently from 737f01f to 7d53424 Compare February 5, 2020 09:26
@rousseldenis
Copy link
Contributor

@simahawk Could you update your review ?

@acsonefho
[10.0][FIX] Fix shopinvader module: Currently when every shopinvader …
70609f3 
…variants related to a shopinvader product are disabled, this shop. product is also disabled (active = False).

But when the shopinvader variant is re-enabled, we should also re-enable the related shop. product (active = True).
Otherwise it could cause some trouble into url management (url.url) who could stay disabled. But it's a side effect
@acsonefho acsonefho force-pushed the 10.0-shopinvader_variant_re_enabled branch from 7d53424 to 70609f3 Compare February 13, 2020 09:01
@shopinvader-git-bot
Copy link

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@simahawk
Copy link
Contributor

/ocabot merge patch

@shopinvader-git-bot
Copy link

This PR looks fantastic, let's merge it!
Prepared branch 10.0-ocabot-merge-pr-568-by-simahawk-bump-patch, awaiting test results.

shopinvader-git-bot pushed a commit that referenced this pull request Feb 13, 2020
Merge PR #568 into 10.0
b98dd47 
Signed-off-by simahawk
@shopinvader-git-bot shopinvader-git-bot merged commit 70609f3 into shopinvader:10.0 Feb 13, 2020
@shopinvader-git-bot
Copy link

Congratulations, your PR was merged at 58be7e7. Thanks a lot for contributing to shopinvader. ❤️

@sbidoul sbidoul deleted the 10.0-shopinvader_variant_re_enabled branch February 13, 2020 15:19
@lmignon lmignon mentioned this pull request Mar 23, 2020
Closed
77 tasks
@HviorForgeFlow HviorForgeFlow mentioned this pull request May 25, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simahawk simahawk simahawk approved these changes

@Cedric-Pigeon Cedric-Pigeon Cedric-Pigeon approved these changes

@rousseldenis rousseldenis rousseldenis approved these changes

Assignees
No one assigned
Labels
10.0 approved enhancement merged 🎉 ready to merge
Projects
None yet
Milestone
10.0
Development

Successfully merging this pull request may close these issues.
6 participants
@acsonefho @codecov-io @rousseldenis @shopinvader-git-bot @simahawk @Cedric-Pigeon