Skip to content

Conversation

@huguesdk
Copy link
Member

module to set point-of-sale orders as to-invoice by default.

@huguesdk huguesdk force-pushed the 16.0-add-pos_auto_invoice branch from 7a93465 to 1131a94 Compare October 30, 2024 11:08
@legalsylvain
Copy link

hi @huguesdk. Thanks for this contribution.

At a first look, LGTM to me.
some remarks :

  • Is it possible to add a boolean auto_invoice_by_default on pos.config and base the change in the PoS regarding that new flag ? Pro :
  • -> it will be installable on multi-company or multi-pos-config context, because such default auto_invoice is great but can be limited to some pos.config.
  • -> we can disable it on demo data, to avoid "excude" mechanism on CI.

What do you think ?

@huguesdk
Copy link
Member Author

i completely agree. like this?

do you think that it should be enabled by default and disabled by demo data? i think that disabled by default makes sense, especially since having a different default behavior with and without demo data could be confusing for users.

@huguesdk huguesdk force-pushed the 16.0-add-pos_auto_invoice branch from 65a5c0e to 6e751e2 Compare October 30, 2024 15:50
@huguesdk huguesdk force-pushed the 16.0-add-pos_auto_invoice branch from 6e751e2 to 32900a4 Compare October 30, 2024 15:51
@legalsylvain
Copy link

do you think that it should be enabled by default and disabled by demo data? i think that disabled by default makes sense, especially since having a different default behavior with and without demo data could be confusing for users

totally agree!

@huguesdk
Copy link
Member Author

huguesdk commented Nov 1, 2024

thanks a lot for the review! i added your suggestion to avoid variable shadowing.

i also ensured that the value of to_invoice is kept for existing orders. strangely, the default behavior in init_from_JSON() is to always set it to false, although it is correctly handled in export_as_JSON().

Copy link

@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 !

@huguesdk huguesdk force-pushed the 16.0-add-pos_auto_invoice branch from 51035ce to 8c4599a Compare November 20, 2024 08:58
Copy link

@polchampion polchampion left a comment

Choose a reason for hiding this comment

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

successful functional test on rotor-v16-test

@github-actions
Copy link

github-actions bot commented Apr 6, 2025

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Apr 6, 2025
@huguesdk
Copy link
Member Author

huguesdk commented Apr 7, 2025

@OCA/pos-maintainers would someone please merge this?

@legalsylvain
Copy link

/ocabot merge nobump

@huguesdk
Copy link
Member Author

huguesdk commented Apr 7, 2025

@legalsylvain thanks! but it doesn’t seem to work. is ocabot sleeping? ☺️

@legalsylvain
Copy link

hi @huguesdk.

Related to OCA/odoo-community.org#204 (comment) maybe ?

@huguesdk
Copy link
Member Author

huguesdk commented Apr 8, 2025

@legalsylvain would you please try again?

@legalsylvain
Copy link

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot merged commit b701632 into OCA:16.0 Apr 8, 2025
5 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved merged 🎉 stale PR/Issue without recent activity, it'll be soon closed automatically.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants