donation icon indicating copy to clipboard operation
donation copied to clipboard

[IMP] donation_base: compute is_donation

Open remytms opened this issue 7 months ago • 1 comments

Add a new field 'is_donation' so that we can easily filter which products are donation and which are not without listing all the values for 'detailed_type'.

This allow to simplify the code, and to easily adds detailed_type for other donation type in other modules.

remytms avatar Jun 04 '25 16:06 remytms

Hi @alexis-via, some modules you are maintaining are being modified, check this out!

OCA-git-bot avatar Jun 04 '25 16:06 OCA-git-bot

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[bot] avatar Nov 16 '25 12:11 github-actions[bot]

@OCA/donation-maintainers could someone please merge this?

huguesdk avatar Nov 17 '25 08:11 huguesdk

I'm not in favor of this change. Why do you need to add a new field ? You have the filter in direct access in the search view of the product...

alexis-via avatar Nov 17 '25 11:11 alexis-via

hi @alexis-via and thanks for the review.

tl;dr: there are 2 reasons for this change:

  1. avoid code duplication by making it easier to check whether a product.template is a donation or not.
  2. allow to more easily add new types of donations.

the first reason and main goal of this change is to put the logic in just one place to avoid code duplication. yes, there are direct-access filters, which is nice for ui users, but for developers, currently, the only way to know if a product.template is a donation (and not a regular product) is to write something like this:

pt.detailed_type in ("donation", "donation_in_kind_consu", "donation_in_kind_service")

or:

pt.detailed_type.startswith("donation")

which is cumbersome, error-prone, leads to a lot of duplication and possibly breaks further extension.

the second reason is to make it easier to extend the module. currently, the list of donation types is hard-coded in 5 places:

  • 3 times in the model class (2 in the field definition and 1 in _detailed_type_mapping()), but that is unavoidable
  • 2 times in the views

that means that if a module wants to add a new type, it will have to modify these 5 places.

the list in the views can easily be avoided thanks to this new field. this is useful, especially since modifying the views to just add an element in the current list is not possible without overwriting the list, which will conflict if there are multiple modules trying to do it.

i just discovered that before version 16.0.2.0.0, there was a boolean donation field! that was actually a good idea, but it was lost in the refactoring.

huguesdk avatar Nov 17 '25 15:11 huguesdk