Skip to content

[18.0] [FIX] quality_control_oca: Handle multiple unlink#1745

Open
diggy128 wants to merge 1 commit intoOCA:18.0from
diggy128:18.0-quality_control_oca_unlink
Open

[18.0] [FIX] quality_control_oca: Handle multiple unlink#1745
diggy128 wants to merge 1 commit intoOCA:18.0from
diggy128:18.0-quality_control_oca_unlink

Conversation

@diggy128
Copy link
Copy Markdown
Contributor

@diggy128 diggy128 commented Mar 2, 2026

  • Fix singleton error when selecting multiple records to delete.
  • Provide a clear reason for failing along with record names that produced the error

@diggy128 diggy128 force-pushed the 18.0-quality_control_oca_unlink branch from 5974c5a to fb7fa43 Compare March 2, 2026 07:53
@diggy128
Copy link
Copy Markdown
Contributor Author

diggy128 commented Mar 2, 2026

@OCA/manufacturing-maintainers
A quick review?

Copy link
Copy Markdown
Contributor

@remi-filament remi-filament left a comment

Choose a reason for hiding this comment

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

Hi @diggy128 thanks for your PR,
a few comments inline

Comment on lines +124 to +128
for record in self:
if record.auto_generated and not record.env.is_superuser():
auto_generated_records |= record
if record.state != "draft":
non_draft_records |= record
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

instead of a loop you could get both list with a .filtered() and do only once self.env.is_superuser() instead of doing it for each record

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment on lines +141 to +142
"Inspections %s are not in draft state and can only be deleted "
"by superuser.",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually I do not think these could even be deleted by superuser since there is no check whether we are superuser or not so it will probably ends up with the same UserError even if superuser...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

True, indeed. Fixed

@remi-filament
Copy link
Copy Markdown
Contributor

Maybe you could also join the 2 errors in one so that the user get the full list of failing inspection tests (otherwise he may remove the auto-generated ones from this list to still get an error with non draft ones)

@diggy128 diggy128 force-pushed the 18.0-quality_control_oca_unlink branch from fb7fa43 to 6cd5576 Compare March 21, 2026 18:51
- Fix singleton error when selecting multiple records to delete.
- Provide a clear reason for failing along with record names that produced the error
@diggy128 diggy128 force-pushed the 18.0-quality_control_oca_unlink branch from 6cd5576 to 38ce326 Compare March 21, 2026 19:38
@diggy128
Copy link
Copy Markdown
Contributor Author

@remi-filament
Done. I also updated the tests. Can you please review?

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