We currently populate fields that currently exist in the url or session in the payments form. We should ensure this behaviour is reflected in Combowiki as well.
Description
Details
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | Laurabarluzzi | T432271 Populate fields from URL/Session values in ComboWiki | |||
| Resolved | Laurabarluzzi | T433537 For ComboWiki, add a helper class to integrate data from get, post, and session | |||
| Resolved | Laurabarluzzi | T433539 Send intial contribution tracking row from ComboWiki |
Event Timeline
Change #1320109 had a related patch set uploaded (by Laurabarluzzi; author: Laurabarluzzi):
[mediawiki/extensions/DonationInterface@master] WIP: integrate fetching data from request and session and handling of contribution tracking in ComboWiki
@Damilare @Cstone @Ejegg I have pushed my branch and the work I have done so far with the input I got.
What I have done:
- Created a class "copy" of DonationData and called it DonationDataHelper and added it as suggested inside the includes/ folder (but under a newly created /ComboWiki/ subfolder). My goal with this was to preserve the existing business logic and data I/O expected from the original class. For now I reduced the dependency from Contribution Tracking logic and I did some basic safe cleanup (e.g. unused private code being removed)
- Created a class ContributionTrackingHelper.php that handles the specific code logic for contribution_tracking queue messages and respective IDs generating. This has a public function 'generateId' that is currently the only dependency for DonationDataHelper (very easy to mock in testing).
- Update ComboWiki.php so that I could wire these two new classes together with the existing gateway adapter inside execute() as discussed. I then passed the data from DonationDataHelper to the client and chose to pass the entire ComboWiki config object at the Vue init.js file (one access point to the global configuration that the components can access by injecting it). This way mw.config can be called only in one place in init.js.
- I added some error handling around the queue push (try/catch + fallbackToDatabase) so a queue failure can degrade gracefully instead of throwing during page load (I was getting warning for unhandled exceptions)
Testing and notes:
- I have successfully validated the wiring and the data locally. Right now my wip branch does nothing with the DonationDataHelper data we get (e.g. overriding default values etc.). I wanted people to first review the solution, address issues with the approach while also seeing how we could use this object to override the current 'routingParams' and default values on page load.
- The message was successfully posted in the contribution_tracking queue; however, because of the current dependency on GatewayAdapter, we do post 2 messages in the queue and one is done by the legacy DonationData which executed automatically on gateway.adapter.php
Todo/reivew:
- Are we happy with the overall approach presented by the current patch? Did I understand things correctly overall?
- Do you see a way to remove even the last dependency of generateId? For this first iteration, my goal was to minimize changes affecting existing code logic. By keeping the same exact behavior, I couldn't see an easy way to remove this dependency.
- Is there a way to easily drop the dependency on GatewayAdapter? Do still want that dependency to be dropped? I recall it was mentioned in a call, but I'm not 100% sure about it being a set goal. If we choose to move forward with the refactoring, I believe something need to change even if only for preventing double messages in the queue.
Please drop any question for discussion here in the task and any code specific question and/or code suggestion in the wip patch. After understanding further the team position on the current solution and options going ways forward, we can discuss detailed over the next steps.
The patch is in final review and it does fetch the data and stores it where it needs it to be for now. Plus it sends the contribution-tracking message to the queue. I created follow-up tasks to address any outstanding, but out of scope, issue with the goal of either improving the code or validating edge cases or improve known wrong behaviors.
The follow-up phabs:
T434835: Clarify usage of DonationDetails dataSources and improve
T434831: Consider improving the wiring of the logger in ComboWiki.php and its classes
T434830: Add tests coverage for the refactored DonationData() code in ComboWiki
T434829: Important: prevent duplicate contribution tracking message in ComboWiki
T434827: Reduce to the minimum all gateway dependencies in ComboWiki.php
T434813: Clarify the types of values of fieldNames passed from DataIntegration() down the chain
T434807: Update DonationInterface\ComboWiki\DataNormalizer::normalizeCountry
T434805: Consider moving block to DataIntegrator()
T434803: Update the list of ::fieldNames in data fetched from request and session
T434804: Define fieldNames allowed in session
T434815: Optional: consider to have a stricter enforcement of value types in DonationDetails()
Change #1320109 merged by jenkins-bot:
[mediawiki/extensions/DonationInterface@master] Integrate fetching data from request and session and handling of contribution tracking in ComboWiki