KYC Verification Report #35765
KYC Verification Report #35765
Conversation
zandre-eng
added
the
product/feature-flag
| success_count = 0 | ||
| fail_count = 0 | ||
| if verify_all: | ||
| # TODO: Need to get all IDS. Could take inspiration from _row_data to fetch all IDs |
There was a problem hiding this comment.
These TODOs are related to the follow-up work that still needs to be completed to implement the actual verification API call and response.
| @@ -32,3 +35,47 @@ class Meta: | |||
| constraints = [ | |||
| models.UniqueConstraint(fields=['domain', 'provider'], name='unique_domain_provider'), | |||
| ] | |||
|
|
|||
|
|
|||
| class KycVerifyTable(BaseHtmxTable): | |||
There was a problem hiding this comment.
@zandre-eng What's your thoughts on putting this table class in a separate tables.py file?
There was a problem hiding this comment.
Great idea! This will help keep things a bit more separate. Addressed in 3508be4.
corehq/apps/integration/kyc/views.py
Outdated
| class KycVerificationReportView(BaseDomainView): | ||
| urlname = 'kyc_verify' | ||
| template_name = 'kyc/kyc_verify_report.html' | ||
| section_name = _("Data") |
There was a problem hiding this comment.
Nit: use single quotes
| section_name = _("Data") | |
| section_name = _('Data') |
There was a problem hiding this comment.
(because the other strings use them and the quotes look out of place)
There was a problem hiding this comment.
Addressed in 64ed0c4.
| <p> | ||
| <button id="verify-selected-btn" class="btn btn-primary" type="button" | ||
| hx-post="{% url 'kyc_verify_table' request.domain %}" | ||
| hx-swap="afterBegin" |
There was a problem hiding this comment.
Since the verify-alert div is empty I was slightly surprised that you didn't use innerHTML here. Is there a particular reason you used afterBegin, since the target element does not have child elements? Or is it to allow for multiple alerts?
There was a problem hiding this comment.
Weirdly, I originally had issues when setting it to innerHTML where the alert ended up popping up in an unexpected place. But testing it now again however, it appears to be working fine. It seems this was something unrelated to the hx-swap value which caused the above issue.
Since it's working now, I've changed it back to innerHTML in 5ed4c59.
|
|
||
| def test_not_logged_in(self): | ||
| response = self._make_request(is_logged_in=False) | ||
| self.assertEqual(response.status_code, 404) |
There was a problem hiding this comment.
Nit: Now that we are using pytest, we can use Pythonic asserts.
| self.assertEqual(response.status_code, 404) | |
| assert response.status_code == 404 |
There was a problem hiding this comment.
I've gone ahead and replaced most of the asserts in this test file with Pythonic asserts in fd522a5 (all apart from the self.assertRedirects one).
| urlname = 'kyc_verify_table' | ||
| table_class = KycVerifyTable | ||
|
|
||
| def get_queryset(self): |
There was a problem hiding this comment.
Sorry, I missed this on my first pass. Doesn't this need to return a QuerySet instance, like the method it overrides?
There was a problem hiding this comment.
The docstring for the get_queryset function that this overrides states:
The return value must be an iterable and may be an instance of
QuerySetin which caseQuerySetspecific behavior will be enabled.
So it can be a QuerySet instance, but as long as it's an iterable then it's okay.
Product Description
A new "KYC Report" page has been added to the "Data" sidebar:
Opening this page reveals a new KYC verification report which can be used to verify users:
Clicking on "Verify All" will verify all the rows across all pages. Selecting one or more rows will cause this button to say "Verify Selected" and, upon clicking it, will only verify the rows that have been selected.
If a row contains invalid and/or missing data, then the invalid columns will be shown as empty (---) and the "Verify" button for that row will be greyed out.
It should be noted that currently clicking on any of the verify buttons doesn't do anything except show a success message on the page. This is because there are followup tickets to implement the actual API call and handle the results thereof.
Technical Summary
Link to ticket here.
Link to design spec here.
Link to tech spec here.
This PR adds a new KYC verification report which will allow users to verify the information of one or more users. This report is able to show either mobile workers or cases, depending on what has been configured on the KYC config page.
This report is currently a read-only report in that the user cannot yet do any actual verification on this page. As mentioned earlier, this will be implemented in follow-up tickets.
Feature Flag
KYC_VERIFICATIONSafety Assurance
Safety story
Automated test coverage
Unit tests exist to ensure that the new views cannot be accessed without authentication or the appropriate feature flag enabled. Additionally, there are also unit tests to make sure that the row data from the report table is correct for both users and cases.
QA Plan
No QA planned.
Rollback instructions
Labels & Review