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

[13.0] Add shopinvader_customer_price + shopinvader_customer_price_wishlist #803

Conversation

simahawk
Copy link
Contributor

@simahawk simahawk commented Nov 4, 2020

Check README for full explanation.

Depends on / includes #807 (hopefully to be merged soon).

@simahawk simahawk added the 13.0 label Nov 4, 2020
@codecov-io
Copy link

codecov-io commented Nov 4, 2020

Codecov Report

Merging #803 (4b04343) into 13.0 (635453a) will increase coverage by 0.01%.
The diff coverage is 94.80%.

Impacted file tree graph

@@            Coverage Diff             @@
##             13.0     #803      +/-   ##
==========================================
+ Coverage   91.90%   91.91%   +0.01%     
==========================================
  Files         145      148       +3     
  Lines        4732     4775      +43     
==========================================
+ Hits         4349     4389      +40     
- Misses        383      386       +3     
Impacted Files Coverage Δ
...vader_customer_price_wishlist/services/wishlist.py 85.00% <85.00%> (ø)
...pinvader_customer_price/services/customer_price.py 96.55% <96.55%> (ø)
...vader_customer_price/models/shopinvader_backend.py 100.00% <100.00%> (ø)
shopinvader/models/shopinvader_product.py 88.77% <0.00%> (-0.23%) ⬇️
shopinvader/models/shopinvader_variant.py 97.50% <0.00%> (-0.21%) ⬇️
shopinvader_lead/services/lead.py 96.00% <0.00%> (-0.16%) ⬇️
shopinvader/services/customer.py 92.18% <0.00%> (-0.13%) ⬇️
shopinvader/services/address.py 93.97% <0.00%> (-0.08%) ⬇️
shopinvader/services/cart.py 87.39% <0.00%> (-0.06%) ⬇️
shopinvader/services/sale.py 100.00% <0.00%> (ø)
... and 16 more

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 635453a...46ff5cb. Read the comment docs.

@simahawk simahawk force-pushed the 13-add-shopinvader_customer_price branch 5 times, most recently from 652866c to aaca01b Compare November 6, 2020 09:31
@simahawk simahawk changed the title [13.0] Add shopinvader_customer_price [13.0] Add shopinvader_customer_price + shopinvader_customer_price_wishlist Nov 6, 2020
@simahawk simahawk force-pushed the 13-add-shopinvader_customer_price branch from aaca01b to e3426f4 Compare November 10, 2020 08:53
@simahawk simahawk force-pushed the 13-add-shopinvader_customer_price branch from e3426f4 to a4a18f6 Compare November 10, 2020 10:07
@lmignon
Copy link
Collaborator

lmignon commented Nov 10, 2020

@simahawk I plan to address this kind of functionality by exporting all the prices into the product index on elasticsearch and use an opendistro addon to restrict the access to part of the json document according to the user connected in the website.....

@simahawk
Copy link
Contributor Author

@simahawk I plan to address this kind of functionality by exporting all the prices into the product index on elasticsearch and use an opendistro addon to restrict the access to part of the json document according to the user connected in the website.....

Great to know it. We'll see if we'll switch to ES sooner or later. This could be a key selling point indeed 😉
Keep me posted 🙏

@lmignon
Copy link
Collaborator

lmignon commented Nov 10, 2020

@simahawk Here is a summary/draft of the approach that we would like to implement and that we have already discussed with @sebastienbeau and @sbidoul . (If one of our lead is confirmed)

  1. Make Odoo an authentication/identity provider for shopinvader customers. The login method would return a JWT token with the identity information of the customer in the payload of the token.
  2. No more use the account management provided by locomotive and rely on the token for the authentication check and the infos of the customer connected. (No more synchronisation of users infos between Odoo and Locomotive backend)
  3. Transmit this JWT token to elasticsearch when querying the indexes in order to make a secure connection to elasticsearch (opendistro module https://opendistro.github.io/for-elasticsearch/features/security.html). The token would contain the role of the user which will allow, in addition to authentication, the use of role-based security rules in elasticsearh.
  4. Set up a mechanism to create/manage security roles in elasticsearch from Odoo based on the role (sale_profile) of shopinvader (a rest api is provided into ES/opendistro). This mechanism would define both the documents to which the user has access and the fields that are visible (it could be made so that the documents returned by elasticsearch only contain the prices of the connected user).

The overall approach would ease and centralise the management of user accounts in Odoo as well as the management of ACL to the products.

@simahawk
Copy link
Contributor Author

simahawk commented Nov 12, 2020

Sounds promising :)

Is there a plan to use redis for session and cache sharing as well?

BTW would you mind tracking this in a specific issue?

@simahawk simahawk force-pushed the 13-add-shopinvader_customer_price branch 7 times, most recently from d12aeda to bd12b11 Compare November 19, 2020 08:11
@simahawk simahawk force-pushed the 13-add-shopinvader_customer_price branch from 1db18fd to cbad677 Compare December 2, 2020 13:01
@simahawk simahawk force-pushed the 13-add-shopinvader_customer_price branch 2 times, most recently from 362a2eb to 98e22da Compare January 26, 2021 09:33
@simahawk simahawk force-pushed the 13-add-shopinvader_customer_price branch 2 times, most recently from 46ff5cb to fa025ae Compare February 10, 2021 15:14
@simahawk simahawk force-pushed the 13-add-shopinvader_customer_price branch from fa025ae to 0173fc8 Compare March 26, 2021 14:45
@rousseldenis rousseldenis added this to the 13.0 milestone Mar 26, 2021
@simahawk
Copy link
Contributor Author

/ocabot merge patch

@shopinvader-git-bot
Copy link

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

@shopinvader-git-bot
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants