newjersey / taxation-mef-viewer

Fork of betson/irs-efile-viewer for DOT use
MIT License
0 stars 0 forks source link

Added support for multiple forms with the same formName #21

Closed jachan closed 1 month ago

jachan commented 1 month ago

Description

Ticket link here

User feedback: "IRS926 – The dashboard correctly shows that there are 5 iterations of this form from the federal XML. However, each instance is a repeat of the first and does not show data from the correct iteration. So the second instance of IRS926 still shows data from the first iteration in the XML, the third instance shows data from the first, and so on. Can verify by looking at the unique values shown in IRS926\CashPropertyGrp\FairMarketValueAmt."

Investigation of ticket https://github.com/newjersey/taxation-mef-viewer/issues/17 revealed that there is a bug in the MeF viewer. When a tax form with multiple instances of the same form (e.g. multiple 926) is viewed, all forms display the data in the first instance of the form.

Approach

This solution requires two parts. First, the page must be updated to differentiate between each instance of a form. Currently, no distinction is made, as all forms are simply referenced by the formName (e.g. 926). This PR changes formId to include a disambiguator (e.g. IRS926_0) when multiple forms with the same name exist.

Second, the function that displays the form when selected must be updated to pull the correct information from the XML DOM. This is done by pulling the index of the form element from the formId disambiguator.

Steps to test

Tested by running locally and validating that I am able to see distinct forms for XML files with multiple 926 forms. Additionally validated that opening each form shows distinct data which matches the XML data.

Current site - note that the two 926 forms are incorrectly displayed as identical to the user:

image image image

Locally Hosted (with fix) - note that the two 926 forms are correct displayed as having unique data:

image image image

Notes

Note: I really want to add a testing framework to this package, especially as I start tinkering with the logic of the transform. Open question: Should that be done in a separate PR or do I bundle it into this one?

jachan commented 1 month ago

Appreciate the feedback from both of you! Looking to get another version out by EOD with all the changes you've mentioned, and I'll try to add some testing just for the feature I'm adding.