Fix the bulk coop report date and distributor filters. #5938
Conversation
Before the date and distributor filters would have no effect. This is because the BulkCoopReport is still generated using an older style method, and isn't generated using the newer method like in the EnterpriseFeeSummaryReport. This older style report expects to receive a :q parameter but it actually received the newer style :report parameter so the filters were not being applied. This keeps the newer style report params but converts them, after they are authorised as safe, into the older style in the controller.
cillian
force-pushed
the
bulk-coop-report-filter-fixes
branch
from
41e456b
to
cdd09e8
Compare
filipefurtad0
added
pr-staged-uk
|
Hi @cillian , I had a look at the Bulk Coop report, before and after the PR. As described in the issue, the date and distributor filters were not working, for the different report types:
This is now working, after this PR:
However, in the pics above, it can be seen that there are change introduced on the columns. I checked for each report types,a found that before this PR, the columns were:
After this PR the columns which appear on all report types are:
So, the filters now work fine, but something seems to be breaking the way the reports are generated. I think this may hide relevant information to the users, so maybe better not to send into production. Could you have a second look at this PR? Thank you! Moving back do InDev. |
…rent report types are generated Also add tests for each of the different report types. I didn't make these JavaScript tests because not sure that is necessary and they would be slower.
…ts but don't check the order. The #table_items methos seems to return line items in different order sometimes making this test a bit flaky. The test passed on Semaphore previously and is passing in development. I don't think the order matters so using :match_array instead of :eq.
|
@filipefurtad0 thanks, sorry I missed that. That should be fixed now, I also added extra tests. |
|
👍 I think one re-review is enough. moving back to test ready. |
|
Hey @cillian , This looks great now:
CSV files are created accordingly. Thank you so much 👍 |
What? Why?
Closes #5837
Before the date and distributor filters would have no effect. This is because the BulkCoopReport is still generated using an older style method, and isn't generated using the newer method like in the EnterpriseFeeSummaryReport. This older style report expects to receive a
qparameter but it actually received the newer stylereportparameter so the filters were not being applied.This keeps the newer style report params but converts them, after they are authorised as safe, into the older style in the controller.
There is a test already for checking that the parameters are authorised correctly here.
What should we test?
Release notes
Fix the bulk coop report date and distributor filters.
Changelog Category: Fixed