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][MIG] shopinvader_api_quotation #1471

Open
wants to merge 10 commits into
base: 16.0
Choose a base branch
from

Conversation

matthieusaison
Copy link

@matthieusaison matthieusaison commented Nov 24, 2023

Continuation of this PR: #1447

In this PR we split model and views in sale_quotation module.
And add shopinvader_sale_state in dependency (#1448)

depend on

@matthieusaison matthieusaison force-pushed the 16.0-mig-shopinvader-api-quotation-2 branch 2 times, most recently from 4d5fd86 to 7b117d1 Compare November 27, 2023 16:45
@matthieusaison matthieusaison force-pushed the 16.0-mig-shopinvader-api-quotation-2 branch from 7b117d1 to 0ee3dd1 Compare November 28, 2023 10:43
@sebastienbeau sebastienbeau force-pushed the 16.0-mig-shopinvader-api-quotation-2 branch 3 times, most recently from 9b3416e to a809af8 Compare December 9, 2023 14:04
@matthieusaison matthieusaison force-pushed the 16.0-mig-shopinvader-api-quotation-2 branch from a809af8 to 23cf892 Compare January 18, 2024 14:32
@matthieusaison
Copy link
Author

👋 Hello @sbidoul, @lmignon can you take a look and merge this PR please.

sale_quotation/readme/DESCRIPTION.rst Outdated Show resolved Hide resolved
shopinvader_api_quotation/routers/cart.py Outdated Show resolved Hide resolved
shopinvader_api_quotation/schemas/sale.py Outdated Show resolved Hide resolved
shopinvader_api_quotation/schemas/sale.py Outdated Show resolved Hide resolved
@sebastienbeau
Copy link
Contributor

@matthieusaison be carefull when you do a rebase, you have drop my commit :'(.
I just re-push them

@sebastienbeau sebastienbeau added this to the 16.0 milestone Jun 3, 2024
matthieu.saison and others added 7 commits June 4, 2024 14:09
- change behaviour so you can use it without shopinvader context
- remove typology in favor of a quotation_state
- rework backend UI to make it more human understandable
- adapt test
@matthieusaison matthieusaison force-pushed the 16.0-mig-shopinvader-api-quotation-2 branch from 6506c60 to 0c7f30f Compare June 4, 2024 12:48
Copy link
Contributor

@sebastienbeau sebastienbeau left a comment

Choose a reason for hiding this comment

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

LGTM (code review). Thanks for the rebase

@sebastienbeau
Copy link
Contributor

@lmignon let's merge it ?

Copy link
Collaborator

@lmignon lmignon left a comment

Choose a reason for hiding this comment

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

Thank you for this proposal. Some comments...

Comment on lines +60 to +62
def _compute_available_for_quotation(self):
for record in self:
record.available_for_quotation = True
Copy link
Collaborator

Choose a reason for hiding this comment

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

@sebastienbeau What's the purpose of this compute method (and the field behind)?

Copy link
Contributor

Choose a reason for hiding this comment

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

The purpose is to make visible or not on the website if the button "request quotation", but we can remove this compute field and simply "inherit" the Schema sale to return something different

@@ -0,0 +1 @@
This module handle the model and view for product and sale.order needs for shopinvader-api-quotation
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should define the context and the 'why' of this addon. This description doesn't provide the minimal information to know when to use it.

shopinvader_api_quotation/routers/cart.py Show resolved Hide resolved
shopinvader_api_quotation/routers/cart.py Outdated Show resolved Hide resolved
shopinvader_api_quotation/routers/cart.py Outdated Show resolved Hide resolved
"quotation_state": "waiting_acceptation",
}
)
with self._create_test_client(router=quotation_router) as test_client:
Copy link
Collaborator

Choose a reason for hiding this comment

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

the quotation_router router is already defined as the default one into your setUpClass method.

Suggested change
with self._create_test_client(router=quotation_router) as test_client:
with self._create_test_client() as test_client:

"quotation_state": "waiting_acceptation",
}
)
with self._create_test_client(router=quotation_router) as test_client:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
with self._create_test_client(router=quotation_router) as test_client:
with self._create_test_client() as test_client:

"quotation_state": "waiting_acceptation",
}
)
with self._create_test_client(router=quotation_router) as test_client:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
with self._create_test_client(router=quotation_router) as test_client:
with self._create_test_client() as test_client:

cart = self.env["sale.order"]._create_empty_cart(
self.default_fastapi_authenticated_partner.id
)
with self._create_test_client(router=cart_router) as test_client:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
with self._create_test_client(router=cart_router) as test_client:
with self._create_test_client() as test_client:

"quotation_state": "waiting_acceptation",
}
)
with self._create_test_client(router=quotation_router) as test_client:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
with self._create_test_client(router=quotation_router) as test_client:
with self._create_test_client() as test_client:

partner: Annotated[ResPartner, Depends(authenticated_partner)],
) -> FileResponse:
"""Download document."""
filename, pdf = (
Copy link
Contributor

Choose a reason for hiding this comment

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

Co-authored-by: Laurent Mignon (ACSONE) <[email protected]>
Comment on lines +13 to +17
from odoo.addons.shopinvader_api_cart.routers import cart_router
from odoo.addons.shopinvader_schema_sale.schemas.sale import Sale


@cart_router.post("/{uuid}/request_quotation")
Copy link
Collaborator

Choose a reason for hiding this comment

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

To not break extension mechanism from odoo, you must define a new router and explain into your readme that your new router must be mounted at the same point as the initial cart router as I did it for the shopinvader_api_sale_loyalty addon in 2c20ea8

@paradoxxxzero
Copy link
Contributor

paradoxxxzero commented Dec 3, 2024

Superseded by #1573

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.

5 participants