dev/financial#197 Revert freezing on total_amount field on recurring form

[eileenmcnaughton]

Overview

dev/financial#197 Revert freezing on total_amount field on recurring form

Before

#21473 results in the amount field being frozen for recurring contributions - it was reviewed as "PASS: The change would be intuitive or unnoticeable for a majority of users who work with this feature." - but that is pretty clearly wrong as users are used to editing the amount, and are unfamiliar with the template contribution - which often does NOT permit editing the amount

After

The field is editable again

Technical Details

This returns the field to the previous state. On checking the code this might mean the recurring amount is out of sync with the template contribution. However, that just takes us back to the status quo which was fine for most users. For single line item contributions the contribution_recur.amount field takes precedence and it doesn't matter if the two are out of sync.

Ideally we need to ensure that any changes to the ContributionRecur.amount are blocked for non-single row or cause a corresponding update for single row templates and this needs to happen regardless of whether the form or the api or BAO makes the update. (It currently IS the case on this form)

Comments

We agreed to underlying schema changes in Barcelona - which have been implemented. Since then UI changes have also been made to add the contribution template to the UI. UI changes definitely need more scrutiny but unfortunately I merged the PR that made this change based on the review, rather than my own analysis, and did not realise it made an important UI change that should have gone through the dev list.

This implements a quick revert on the problematic part (as suggested by Matt). We can re-discuss a fix in master but after testing this out I don't think the freeze is acceptable while we do this

@KarinG @mattwire @adixon @seamuslee001

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[mattwire]

@eileenmcnaughton I'd prefer if we just commented out the freeze rather than removing the code. I know it's available in git history (if you can find it) but we're likely to revisit this soon and I'd like the next developer (me included) to remember why we can't simply freeze that field.

[eileenmcnaughton]

Happy for you to put up an alternate version that you find clearer

[mattwire]

See #23805