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] delivery_state_manual #924

Merged

Conversation

Tisho99
Copy link
Contributor

@Tisho99 Tisho99 commented Dec 31, 2024

This module extends the functionality of delivery_state to allow you to manually edit the delivery state of pickings with specific delivery carriers

T-7072

@Tisho99 Tisho99 force-pushed the 16.0-add-delivery_state_manual branch from 7be38f5 to 7af67a5 Compare January 2, 2025 08:53
@Tisho99 Tisho99 marked this pull request as ready for review January 2, 2025 08:54
@Tisho99
Copy link
Contributor Author

Tisho99 commented Jan 2, 2025

@Tisho99 Tisho99 force-pushed the 16.0-add-delivery_state_manual branch from 7af67a5 to cade345 Compare January 2, 2025 09:06
Copy link

@Jaimermaccione Jaimermaccione left a comment

Choose a reason for hiding this comment

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

@Tisho99 Hi Alberto!

Hi Alberto!

There’s a behavior regarding the setting of the delivery date (date_delivered) that, as I see it, is currently set at the moment in the "shipping_recorded_in_carrier" and "customer_delivered" states. I think it would be more appropriate for "shipping_recorded_in_carrier" to remain hidden and for the date to be set and visible in the "warehouse_delivered" state, which is essentially another type of delivery.

Let me explain: the "shipping_recorded_in_carrier" state is meant to represent the first step, where the goods are handed over to the person responsible for transporting them to the client, whether it’s an agency or an in-house driver. The delivery date should be set when the goods have reached their destination, and I believe the two states I mentioned align better with that timing.

If we decide to make this change, we should also update the README accordingly. Just something to keep in mind. What do you think?

@rousseldenis rousseldenis added this to the 16.0 milestone Jan 8, 2025
@Tisho99 Tisho99 force-pushed the 16.0-add-delivery_state_manual branch from cade345 to db3a2eb Compare January 8, 2025 09:31
@Tisho99
Copy link
Contributor Author

Tisho99 commented Jan 8, 2025

@Jaimermaccione

Okay, i see it well

I replaced the "shipping_recorded_in_carrier" state for the "warehouse_delivered" state in the code

@Tisho99 Tisho99 force-pushed the 16.0-add-delivery_state_manual branch 2 times, most recently from af05058 to b6b922a Compare January 9, 2025 10:48
@Tisho99 Tisho99 force-pushed the 16.0-add-delivery_state_manual branch from b6b922a to 4206858 Compare January 9, 2025 11:18
Copy link

@Jaimermaccione Jaimermaccione left a comment

Choose a reason for hiding this comment

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

@Tisho99

LGTM! Tested in local enviroment.

@Tisho99 Tisho99 force-pushed the 16.0-add-delivery_state_manual branch from 4206858 to ac8404f Compare January 14, 2025 15:24
Copy link

@manuelregidor manuelregidor left a comment

Choose a reason for hiding this comment

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

Technical review. LGTM

Copy link
Contributor

@ValentinVinagre ValentinVinagre left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏻

@OCA-git-bot
Copy link
Contributor

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

@HaraldPanten
Copy link

@etobella Do you mind merging this one?

All the comments have been considered.

THX!

@etobella
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-924-by-etobella-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 8eb5656 into OCA:16.0 Jan 14, 2025
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 1ca0ccb. Thanks a lot for contributing to OCA. ❤️

@HaraldPanten HaraldPanten deleted the 16.0-add-delivery_state_manual branch January 15, 2025 08:12
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.

8 participants