compucorp / uk.co.compucorp.membershipextras

Membership Extras for CiviCRM
Other
5 stars 8 forks source link

MAE-380: Membership type rolling period instalment payment plan new screen #305

Closed erawat closed 3 years ago

erawat commented 3 years ago

Overview

This PR is to simplify the payment plan on the "New Membership" screen.

The instalments are generated by selected schedule or selected membership type.

Before

The existing instalment screen was removed and already merged to base branch so the instalment was not existed in this case.

After

When select membership type and/or schedule the instalment will be displayed based on schedule.

Peek 2020-10-28 10-47

Currently, the new screen does not support fixed membership type and price set.

Technical Details

This PR has refactored how instalments calculated from calculating the instalment on the font end (JavaScript) to calculate instalments from the back end.

What have been changed?

The calculation and format are done in the back end so the front end would only need to generate the display.

{"is_error":0,"version":3,"count":4,"values":[{"instalment_no":"1","instalment_date":"25th October 2020","instalment_tax_amount":"\u00a30","instalment_amount":"\u00a330","instalment_status":"Pending"},{"instalment_no":"2","instalment_date":"25th February 2021","instalment_tax_amount":"\u00a30","instalment_amount":"\u00a330","instalment_status":"Pending"},{"instalment_no":"3","instalment_date":"25th June 2021","instalment_tax_amount":"\u00a30","instalment_amount":"\u00a330","instalment_status":"Pending"},{"instalment_no":"4","instalment_date":"25th October 2021","instalment_tax_amount":"\u00a30","instalment_amount":"\u00a330","instalment_status":"Pending"}]}

Comment

There is one unit test fails but not related to this update and it will be fixed separately.

erawat commented 3 years ago

So I left some notes but I also have some other generic notes/questions :

1- The commit messages and their order and what each commit contain is really a mess, the first commit, for example, contains a lot of debugging and commented out code, can you maybe reorganize them more properly, interactive rebase can help with that maybe or whatever method you prefer, also commit messages like "MAE-380: Rename classes to reflect what they do" or "MAE-380: Format the function" does not reflect the actual purpose of your PR.

Commits will be squashed to make it cleaner.

2- What about fixed membership types and price sets, are you planning to start working on them soon, I am saying that because they already part of the ticket scope (MAE-380), I am fine if you are doing them in another PR (or more preferably creating a new ticket for them) as long as they are not forgotten.

This will be done in another PR, perhaps with sub ticket for this as I want this PR to be reviewed, so we could agree on the approach before implementing fixed period membership type.

3- I am not sure if we decided to support quarterly payment plans or not, maybe something to confirm with Ana since she was planning to check on that.

I won't support quarterly payment for fixed membership type but for rolling membership type, we are supporting them.

4- Not sure if this is part the specs documentation, but what about "manual" renewal screen does this work with it? maybe to be checked with Ana too.

Will check.