Add Generate Prepayment Invoice to Accounting Admin - #38125
Conversation
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The form decides whether to generate or schedule from the send date. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Use new GeneratePrepaymentInvoiceForm, which subclasses the existing WirePrepaymentForm but with the addition of a 'domain' field.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
nospame
added
the
product/admin
| assert all(value in text_content for value in data.values()) | ||
|
|
||
|
|
||
| def wire_prepayment_post_data(**overrides): |
There was a problem hiding this comment.
nit: The newspaper principle would put this utility below the tests that use it.
| def scheduled_prepayment_post_data(**overrides): | ||
| data = { | ||
| 'email_to': 'billing@example.com', | ||
| 'email_cc': 'ap@example.com', | ||
| 'credit_label': '12 month prepayment', | ||
| 'unit_cost': '1000.00', | ||
| 'quantity': 12, | ||
| 'amount': '12000.00', | ||
| 'prepay_date_start': '2027-01-01', | ||
| 'prepay_date_end': '2028-01-01', | ||
| 'send_date': in_days(90).isoformat(), | ||
| } | ||
| data.update(overrides) | ||
| return data |
There was a problem hiding this comment.
Was going to suggest reducing duplication here, but I see the defaults are quite different in wire_prepayment_post_data so maybe it would require too much test rewriting? Up to you if you want to use this or some variation of it.
| def scheduled_prepayment_post_data(**overrides): | |
| data = { | |
| 'email_to': 'billing@example.com', | |
| 'email_cc': 'ap@example.com', | |
| 'credit_label': '12 month prepayment', | |
| 'unit_cost': '1000.00', | |
| 'quantity': 12, | |
| 'amount': '12000.00', | |
| 'prepay_date_start': '2027-01-01', | |
| 'prepay_date_end': '2028-01-01', | |
| 'send_date': in_days(90).isoformat(), | |
| } | |
| data.update(overrides) | |
| return data | |
| def scheduled_prepayment_post_data(*, send_date=None, **overrides): | |
| return wire_prepayment_post_data( | |
| send_date=send_date or in_days(90).isoformat(), | |
| **overrides | |
| ) |
There was a problem hiding this comment.
Good point, addressed this and the above here: b7a9dc0
| return self.cleaned_data['prepay_date_end'] or datetime.date.today() | ||
|
|
||
| def clean_credit_label(self): | ||
| credit_label = self.cleaned_data.get('credit_label', 'General Credits') |
There was a problem hiding this comment.
Potential blocker: Could this return an empty string if the credit_label field was present but had no content?
| credit_label = self.cleaned_data.get('credit_label', 'General Credits') | |
| credit_label = self.cleaned_data.get('credit_label') or 'General Credits' |
There was a problem hiding this comment.
As Graham pointed out, this actually wasn't necessary at all, because it's handled by the form field's empty_value. 7205503
| name=CreditsWireInvoiceView.urlname), | ||
| url(r'^subscription/credits/schedule_prepayment_invoice/$', | ||
| SchedulePrepaymentInvoiceView.as_view(), | ||
| name=SchedulePrepaymentInvoiceView.urlname), |
There was a problem hiding this comment.
Potential blocker: There will be some time after this is deployed where pages that were loaded with the previously deployed code may submit to this URL, and that would result in a 404. It does seem annoying to keep this around until the next deploy, greatly reducing the chances that someone would access this URL (although it's still not impossible in that scenario either).
Do we care?
There was a problem hiding this comment.
I don't think we would care, because while technically customer-accessible, this feature is only ever used in practice by our own staff - who will probably be using the new "Generate Prepayment Invoice" option now. It's obviously not ideal if they can hit a temporary 404, but it's easy enough to just ask them to refresh.
gherceg
left a comment
gherceg
left a comment
There was a problem hiding this comment.
Holding off on approval just to give some time to respond to comments, but this looks close to ready to me.
DRY scheduled_prepayment_post_data and organize helpers after tests that use them
The form field's empty_value='General Credits' handles this itself.
3 participants
Product Description
Adds a link to Generate Prepayment Invoice in the accounting admin sidebar, and loads the same form modal that is used on the domain-specific "Current Subscription" page - with the addition of a project space name.
Technical Summary
https://dimagi.atlassian.net/browse/SAAS-19996
Moves prepayment invoice validation and generation into a django form and adds this to a new accounting admin view. The form determines whether to create the prepayment invoice now, or create a scheduled invoice that will generate and send the prepayment invoice later. The form is re-used between the customer-facing Current Subscription page and the new accounting admin-facing Generate Prepayment Invoice page.
There is unfortunately a small amount of new knockout code added on this page. Without fully rewriting or duplicating the existing "credits" modal, which is what shows the Generate Prepayment Invoice form, there was no good way around this.
Safety Assurance
Safety story
There is some relatively significant refactoring here but it is well-tested, including some view tests - which pass first and then are removed in a later commit. While customer-facing, this feature is really only used by accounting admin staff, so its impact would be somewhat limited. Tested locally.
Automated test coverage
Automated testing is added for the new form validation, some of which is moved from elsewhere, and ultimately most of the view tests are removed as they were looking at form logic more than view rendering.
QA Plan
Not planning it.
Rollback instructions
Labels & Review