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] website_sale_commitment_date #1004

Open
wants to merge 1 commit into
base: 16.0
Choose a base branch
from

Conversation

adasatorres
Copy link

Hi, this addon allows selecting the delivery date when purchasing from the e-commerce platform, configuring excluded days of the week, minimum or maximum days to limit the selection
from the current date. All of this is configurable from the shipping method.

@adasatorres adasatorres force-pushed the 16.0-add-website_sale_commitment_date branch 4 times, most recently from 66af92e to c03219e Compare January 14, 2025 09:00
@adasatorres
Copy link
Author

To use this addon, you need to go to the shipping method and activate the "allow commitment date" option.
Then, you can configure various settings right there. Afterward,
go to the cart in the e-commerce platform, and you will see that if you select that shipping method,
the option to choose the delivery day will appear.
When using a shipping method that requires specifying the delivery date,
the button will not appear until a date is selected.

Copy link

@rrebollo rrebollo left a comment

Choose a reason for hiding this comment

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

Code Review: Great work! The code looks good to me (LGTM). Thank you for your contribution! I've provided a few suggestions for your consideration—feel free to address them as you see fit. Btw, nice tests.

Comment on lines +34 to +40
lang = request.env["res.lang"]._lang_get(
request.env.context.get("lang", False) or request.env.user.lang
)
if order:
date_obj = datetime.strptime(
string_date,
lang.date_format if lang else DEFAULT_SERVER_DATE_FORMAT,

Choose a reason for hiding this comment

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

Perhaps you can use "from odoo.tools.misc import format_date" to the date manipulation according to language and encapsulate this behavior in a separated auxiliary method to reuse below too.

Copy link
Author

Choose a reason for hiding this comment

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

Hi @rrebollo, I believe that format_date cannot be used to create a date object from a string.

Choose a reason for hiding this comment

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

Of course, my suggestion implicitly mean to do some refactoring around to rely upon odoo's built-in wrappers to work with python dates. But it was only a suggestion. Again, great work

def _compute_name(self):
for record in self:
if record.value:
record.name = record.value.strftime("%d/%m/%Y")

Choose a reason for hiding this comment

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

Why ""%d/%m/%Y" as format? Wouldn't "%Y/%m/%d" be "safer" instead?

Copy link
Author

@adasatorres adasatorres Feb 14, 2025

Choose a reason for hiding this comment

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

Hi @rrebollo, I could use the date format provided by the language, so the format will be created based on the user's language.

Choose a reason for hiding this comment

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

Yes that'll be a better approach in deed

@adasatorres adasatorres force-pushed the 16.0-add-website_sale_commitment_date branch from c03219e to 51a9269 Compare February 21, 2025 13:07
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.

2 participants