Skip to content

[16.0][IMP]pos_order_to_sale_order: Allow add the commitment date. #1361

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

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

Conversation

adasatorres
Copy link
Contributor

This add-on allows you to add the commitment date to orders.
I've included a reference to the previous PR to maintain continuity of the discussion #1360 .

@OCA-git-bot
Copy link
Contributor

Hi @legalsylvain,
some modules you are maintaining are being modified, check this out!

@adasatorres adasatorres force-pushed the 16.0-imp-pos_order_to_sale_order branch 3 times, most recently from faeb18a to a3c4ce2 Compare April 8, 2025 16:38
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.

Great job! The code looks good to me (LGTM). Thanks a lot for your contribution! 🙌
Just one thought: I see quite a bit of code around timezone handling (if I'm not mistaken). Do you think context_timestamp could already cover your needs, at least on the Python side?

Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

Thanks a lot ! Looks more simple.

One question : finally, don't see clearly the interest of the new setting "iface_sale_order_allow_commitment_date".

i mean, this field is optional, so it's possible for the use to enter the date, or let empty. don't you think ?

@Christian-RB
Copy link

The concern seems to be that the view could become too crowded in the future, so IMO keeping this as an option is more flexible for the users

@adasatorres adasatorres force-pushed the 16.0-imp-pos_order_to_sale_order branch 2 times, most recently from ec869f8 to 79e81a8 Compare April 10, 2025 19:30
@adasatorres adasatorres force-pushed the 16.0-imp-pos_order_to_sale_order branch from 79e81a8 to 8c94303 Compare April 10, 2025 19:31
Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

The concern seems to be that the view could become too crowded in the future, so IMO keeping this as an option is more flexible for the users

OK.

I guess it's ready to be merged, as soon as the date problem is fixed.

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