-
-
Notifications
You must be signed in to change notification settings - Fork 94
[IMP] donation_base: compute is_donation #141
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
base: 16.0
Are you sure you want to change the base?
Conversation
|
Hi @alexis-via, |
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'.
5a79fb2 to
4160e35
Compare
|
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. |
|
@OCA/donation-maintainers could someone please merge this? |
|
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... |
|
hi @alexis-via and thanks for the review. tl;dr: there are 2 reasons for this change:
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 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:
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 |
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.