-
-
Notifications
You must be signed in to change notification settings - Fork 416
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
[14.0][IMP] account_statement_import_camt54 add the parameter to the view #607
base: 14.0
Are you sure you want to change the base?
Conversation
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. |
@alexis-via Willing to review this small PR? |
@OCA/banking-maintainers Anyone willing to review? |
I'm really disappointed that this feature was added on the camt54 level rather than generically. This aspect is missing from the field's help text. It suspiciously uses Are you using this feature? Otherwise I'd say maybe keep it hidden until it's improved. |
bc444f4
to
fcb637c
Compare
Yes we think this feature is useful and are using it. I made a few changes based on your comments. I'm not convinced the feature would be useful at the generic level though. |
Thanks for addressing my rant so gracefully! Please use the prefix |
The parameter wasn't added to the view although it was defined in the model.
- Use currency.iz_zero() instead of direct comparison - Add information about camt54 usage in the help of the field
fcb637c
to
0301ca7
Compare
You're welcome 😉 |
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. |
The
transfer_line
parameter wasn't added to the view although it was defined in the model.