Skip to content

[18.0][ADD] sale_confirm_group: Add module #3953

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: 18.0
Choose a base branch
from
Open

[18.0][ADD] sale_confirm_group: Add module #3953

wants to merge 1 commit into from

Conversation

@SilvioC2C
Copy link

@SilvioC2C SilvioC2C commented Oct 17, 2025 •
edited by simahawk
Loading

This module allows configuring a list of groups per-company who are granted permission to confirm sale orders:

  1. button "Confirm" in sale views is always hidden for users not in those groups
  2. if users outside those groups try to confirm a SO, an error is raised

@SilvioC2C SilvioC2C force-pushed the ADD-sale_confirm_security_group branch 2 times, most recently from 19edba1 to 30cb0f1 Compare October 17, 2025 20:51
@rousseldenis rousseldenis added this to the 18.0 milestone Oct 18, 2025
rousseldenis
rousseldenis reviewed Oct 18, 2025
View reviewed changes
@SilvioC2C SilvioC2C force-pushed the ADD-sale_confirm_security_group branch from 30cb0f1 to 1f6b48e Compare October 20, 2025 00:07
@SilvioC2C SilvioC2C changed the title [18.0][ADD] sale_confirm_security_group: Add module [18.0][ADD] sale_confirm_groups: Add module Oct 20, 2025
@SilvioC2C SilvioC2C force-pushed the ADD-sale_confirm_security_group branch 2 times, most recently from 2fb805c to f1f4454 Compare October 20, 2025 00:17
@SilvioC2C SilvioC2C force-pushed the ADD-sale_confirm_security_group branch from f1f4454 to a83691b Compare October 20, 2025 08:29
@SilvioC2C
Copy link
Author

Tests are failing because of a 503 error, possibly due to AWS outage

simahawk
simahawk requested changes Oct 20, 2025
View reviewed changes
@simahawk
Copy link
Contributor

The name should be sale_confirm_group

@SilvioC2C SilvioC2C force-pushed the ADD-sale_confirm_security_group branch from 9e6b60e to 739a66a Compare October 20, 2025 16:12
@SilvioC2C
Copy link
Author

CI is red due to odoo_test_helper.FakeModelLoader._check_wrong_import() that thinks models were already loaded, if someone can give an hint I'd be happy to check it

simahawk
simahawk requested changes Oct 21, 2025
View reviewed changes
sale_confirm_group/tests/test_sale_confirm_group_views.py Outdated
# - a dummy function ``action_confirm()`` on ``res.users``
# so we can add nested list views w/ other ``action_confirm`` buttons
cls.loader = FakeModelLoader(cls.env, cls.__module__)
cls.loader.backup_registry()
Copy link
Contributor

Choose a reason for hiding this comment

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

you miss the restore_registry call

sale_confirm_group/tests/test_sale_confirm_group_views.py Outdated
cls.env = cls.env(context=dict(cls.env.context, load_all_views=True))

# For button-invisibility testing we add:
# - a dummy M2M field from ``sale.order`` to itself
Copy link
Contributor

@simahawk simahawk Oct 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

this complexity comes from the complexity on get_view that we discussed and should be removed

@simahawk
Copy link
Contributor

The name should be sale_confirm_group

reminder

@simahawk simahawk changed the title [18.0][ADD] sale_confirm_groups: Add module [18.0][ADD] sale_confirm_group: Add module Oct 22, 2025
@simahawk simahawk force-pushed the ADD-sale_confirm_security_group branch from 739a66a to c5b352b Compare October 22, 2025 13:35
@simahawk
Copy link
Contributor

I've cleaned up the complexity

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

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

Reviewers

@simahawk simahawk simahawk approved these changes

@rousseldenis rousseldenis Awaiting requested review from rousseldenis

+1 more reviewer

@henrybackman henrybackman henrybackman approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approved needs review ready to merge

Projects

None yet

Milestone

18.0

Development

Successfully merging this pull request may close these issues.

5 participants

@SilvioC2C @simahawk @OCA-git-bot @henrybackman @rousseldenis