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

[16.0][ADD] addon shopinvader_api_address #1361

Merged
merged 2 commits into from
Jul 30, 2023

Conversation

AnizR
Copy link
Contributor

@AnizR AnizR commented Jun 6, 2023

Depends on OCA/rest-framework#348 for tests.

Addon to manipulate shipping/billing addresses linked to authenticated_partner

  • The authenticated_partner is a shipping and a billing address. Thus, there will always be at least one billing and shipping address when there is an authenticated_partner (given the fact that the address is complete/has enough information to be a shipping/billing address) will be done in front of shopinvader
  • Add fastapi tags

The billing address is unique and is the authenticated_partner. Therefore, it is impossible to delete it
The shipping addresses have no restriction

  • Billing Address
    • create/update: it will update authenticated_partner
    • get
    • (search: no need)
    • (delete: no need)
  • Shipping Address
    • create
    • update
    • get
    • search : in get overkill
    • delete: will archive
  • readme
  • small addon to show how add field on address : shopinvader_api_address_shipping_note / shopinvader_address_shipping_note

@AnizR AnizR marked this pull request as draft June 6, 2023 10:41
@AnizR AnizR force-pushed the acsone/16.0-api-address branch 6 times, most recently from 116320d to c48067e Compare June 7, 2023 10:23
@sebastienbeau sebastienbeau added this to the 16.0 milestone Jun 9, 2023
shopinvader_api_address/routers/address_service.py Outdated Show resolved Hide resolved
shopinvader_api_address/schema/schema.py Outdated Show resolved Hide resolved
shopinvader_api_address/models/res_partner.py Outdated Show resolved Hide resolved
shopinvader_schema_address/schema/address.py Outdated Show resolved Hide resolved
@AnizR AnizR changed the title [ADD] addon schema_address: init [WIP][ADD] addon shopinvader_api_address Jun 22, 2023
shopinvader_address/models/res_partner.py Outdated Show resolved Hide resolved
shopinvader_api_address/routers/address_service.py Outdated Show resolved Hide resolved
shopinvader_api_address/routers/address_service.py Outdated Show resolved Hide resolved
shopinvader_api_address/routers/address_service.py Outdated Show resolved Hide resolved
shopinvader_api_address/routers/address_service.py Outdated Show resolved Hide resolved
<!-- allow authenticated partner to snailmail_letter where it is used-->
<record id="shopinvader_address_snailmail_letter_rule" model="ir.rule">
<field name="name">Shopinvader Address Endpoint rule: snailmail_letter</field>
<field name="model_id" ref="snailmail.model_snailmail_letter" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To investigate. If this rule stays then shopinvader_api_address must depend on snailmail, which would be... not good.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I will remove it.

I added this because a check is done in snailmail in write of res.partner : https://github.com/odoo/odoo/blob/a4e3662351420b9b87eac386d6bd7b4f075d9fa9/addons/snailmail/models/res_partner.py#LL20C9-L20C9

The problem is that the access to the model snailmail_letter is given to only base.group_user and base.group_system. In our case, we don't want to give any of those group to the endpoint's user (too much rights).

(IMO, an issue can (and will) be created on Odoo regarding the fact that the check should be done in sudo() + I think that the fact that every user can read every snailmail_letter is also an issue.. )

To tackle this problem, I think that the best option is to call in sudo mode our functions that does a write on res.partner in our services (in the module shopinvader_api_address).

@sbidoul sbidoul marked this pull request as ready for review June 26, 2023 16:59
@sbidoul
Copy link
Member

sbidoul commented Jun 26, 2023

This is the v2 address service, ready for final review. @sebastienbeau @thibaultrey @hparfr

image

A few notes:

Copy link

@FrancoMaxime FrancoMaxime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

shopinvader_address/models/res_partner.py Show resolved Hide resolved
shopinvader_address/models/res_partner.py Outdated Show resolved Hide resolved
shopinvader_api_address/__manifest__.py Outdated Show resolved Hide resolved
shopinvader_address/models/res_partner.py Outdated Show resolved Hide resolved
shopinvader_address/models/res_partner.py Outdated Show resolved Hide resolved
@sbidoul sbidoul changed the title [WIP][ADD] addon shopinvader_api_address [16.0][ADD] addon shopinvader_api_address Jun 29, 2023
author Zina Rasoamanana <[email protected]> 1686047094 +0200
committer Zina Rasoamanana <[email protected]> 1688136067 +0200

[ADD] addon shopinvader_schema_address: init

wip: add addres_billing/shipping get/create

update shipping address

work on tests + use country/state_id + work on address schemas

rename address service

extract service logic from model

extract shopinvader_address

some improvements + sudo on service write

add vat on billing address

split create/update shipping address

check before update/delete shipping address

readme shopinvader_schema_address

shopinvader_api_address readme

shopinvader_address: readme

[FIX] shopinvader_api_address_*: Uses new helper class to run tests with fastapi and extendable
@AnizR
Copy link
Contributor Author

AnizR commented Jun 30, 2023

I cleaned up my commits history @xavier-bouquiaux @marielejeune, you may need to rebase your branches

@marielejeune
Copy link
Contributor

marielejeune commented Jul 26, 2023

Main changes concern the following modification:

Pydantic v2 now makes a distinction between nullable fields and required fields.

vat: str | None makes the vat attr nullable but required. Hence we must add default None value (= None) to make it not required.

See here for more details

@lmignon
Copy link
Collaborator

lmignon commented Jul 26, 2023

/ocabot merge nobump

@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). 🤖

1 similar comment
@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). 🤖

@shopinvader-git-bot
Copy link

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-1361-by-lmignon-bump-nobump, awaiting test results.

@shopinvader-git-bot shopinvader-git-bot merged commit 993375a into shopinvader:16.0 Jul 30, 2023
3 checks passed
@shopinvader-git-bot
Copy link

Congratulations, your PR was merged at b9da4d8. 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.

7 participants