-
Notifications
You must be signed in to change notification settings - Fork 4
Major TODOs
Beyond the TODOs in the code there are some grander TODOs that ought to be addressed, so below are some notes on each (context, rationale and some ideas for implementation) [not in priority order]...
Canned members-data-api
responses, for both local dev and to facilitate StoryBook & ChromaticQA (visual regression testing)
Whilst we have some very basic Jest snapshot testing of top-level components, we really could do with a more comprehensive visual regression testing solution. Storybook and ChromaticQA have proved super useful for other teams so it makes total sense to introduce them in manage-frontend
.
The main challenge would be to have some canned members-data-api
responses to feed lots of higher level components to ensure such a solution would be comprehensive enough to catch regressions. This could be used for local development too and would mean instant responses (can be quite painful right now with lots of subs - as DEV/CODE members-data-api
is quite slow - because Zuora is slow) we could build up quite a suite of canned responses to exemplify the multitude of product scenarios and subscription states that manage-frontend
supports.
Whilst we use Sentry (client-side and server-side) for catching errors introduced by PRs, it's high time we do a bit of an audit of all outstanding issues in both Sentry projects, to either...
- fix the problems (there will no doubt be some genuine issues)
- change the error handling such that it's captured more gracefully
- if it's truly out of our control (network issues on their device for example) then mark as 'Ignore' in Sentry UI (probably also be worth exploring the settings in Sentry UI [needs admin] to see if there's any 'known error'/AI magic to dismiss any noise automatically).
Right now we only display recurring contributions (as they come from Zuora via members-data-api
) whereas single contributions need to be obtained from the contribution-store
- fortunately members-data-api
already has integration for this (primarily via members-data-api/pull/388) for the purposes of recognising one-off contributors on the main website - however in order to display the required detail in MMA would need a new members-data-api
endpoint.
This feature exists in the current designs but has yet to be prioritised, presumably because the user can't actually 'manage' anything about the contribution(s) as they're, well, 'one-off'. Once implemented though it will be better UX as well as providing a quick way for customers to contribute again, or better yet start a recurring contribution.
migrate from node-riffraff-artefact
to node-riffraff-artifact
manage-frontend
still relies on the legacy node-riffraff-artefact
for its riff-raff integration and artifact creation. This really ought to be upgraded to use node-riffraff-artifact
- should be easy enough.
manage-frontend
reads the S3 files produced by fulfilment-date-calculator
in order to display expected dates that delivery address changes will become effective (see wiki/Delivery-Address-Update-Flow). These dates are spliced into the members-data-api
response on the Node backend before it makes it to the client (see wiki/Proxying-members-data-api), really this should just be done in members-data-api
- can't recall why we did it in manage in the first place (probably for speed/ease).
In the beginning we were asked to replicate the footer on the main website, for consistency as we migrated functionality into manage-frontend
from various places. However it has since had little attention and so is probably out of date. Conversations have been had about making it a design systems component but no firm plan is in place. I would advocate that we diverge from dotcom (as there's to much on there for MMA needs) and converge on a 'skinnier' footer like that in support-frontend
- making it part of the RR layer of Design Systems. That said, back in previous re-brands where a skinner footer was posed, there was push-back from design - but worth raising again (to move code/complexity out of manage 🙂 ).
Currently, we run pretty-quick --staged
as one of the git pre-commit hooks to ensure all changed code is run through prettier
before making it into VCS. However, we don't have the prettier
rules explicitly codified anywhere, and so presumably it uses the default values of pretty-quick
. This becomes annoying when using prettier
extensions in IDEs as they use the default rules of prettier
which are slightly different.
So we should somehow extract the default rules of pretty-quick
and codify into a config file (e.g. .prettierrc
) which can be used by both pretty-quick
AND prettier
IDE extensions/plugins.
Although not purely a manage-frontend
TODO, the wiki/Delivery-Address-Update-Flow is more complicated as a result, due to the fact that Delivery Addresses are poorly modelled - they're on the Salesforce contact which means subs can't go to different delivery addresses - which means we need convoluted UI to explain this customer (among other issues).
This is a big piece of work, but delivery addresses should be moved to their own entity (most likely a new object in Salesforce, which is then linked to the 'Contact' and the 'SF Subscription'), which for MMA would facilitate the more intuitive idea of an 'Address Book' where given an address, users could simply tick the subscriptions they'd like it to be used for. Right now we don't support self-service delivery updates for customers with multiple contact IDs on their subs (which most notably occurs for gift subs).
The fulfilment-lambdas will need to be updated as part of this work, but there's already a proposal to fully rewrite that, see fulfilment-lambdas/adr/simplified_architecture.md.
We've long wanted to offer the ability for customers to switch payment method themselves (in fact there's a hidden payment method switching section in the Payment Update Flow as a placeholder) and the re-design defines a nice tick box approach for changing payment methods as well as the concept of a 'wallet' (like virtually all online ecommerce platforms).
To get there...
-
members-data-api
would need to always serve the correct stripe public key for each sub (currently it's only present if the customer is already using card to pay) - we'd probably need an additional endpoint in members-data-api which groups the duplicate payment methods across a customers subs, very similar to the /user-attributes/me/existing-payment-options endpoint, to facilitate the 'wallet' idea
- the current payment method update endpoints would need to first ensure that the billing accounts get put in the right state (updating the 'Payment Gateway' etc.) but also that they take a list of sub IDs to apply the new payment method onto, to facilitate the tick-boxes in the Payment Update Flow (it's possible to use the same Stripe tokens on multiple billing accounts, however the GoCardless mandates will need to be 'cloned' to ensure they appear separately in customers online banking etc, see support-frontend/wiki/Payment-Method-Re-use)
As per Server-side Logging & Metrics we have metrics on all of our critical paths (and more) so it would be wise to add some alarm thresholds on these, such as 'no payment updates in the last hour'.
One could use the CloudWatch dashboard (called manage-frontend
) to work out what the different thresholds would be.
This will help us catch/understand outages in the systems we depend on, but also to catch any regressions in manage-frontend
(or indeed any of our other systems).
TODO
TODO (https://github.com/guardian/support-frontend/wiki/Payment-Method-Re-use)
TODO (https://github.com/guardian/support-frontend/wiki/Payment-Method-Re-use
identity sign-in flow for LastName/SubID (so /manage
on subs platform can be fully retired) with nice linking mechanism
TODO
TODO
TODO (notably content security policy & CSRF)
TODO (could do this via Fastly)
TODO
TODO
TODO
TODO
Although not purely a manage-frontend
TODO, this would simplify how test-users
work in manage (see wiki/test-users) by not having to rely on the X-Gu-Membership-Test-User
header from members-data-api
to then send on to other APIs.
It would be much better if we abandoned the whole 48hour special strings (that can be produced on the /test-user
endpoints of our acquisitions platforms) which require library dependencies on so many of our systems and are a constant source of confusion for many people. Instead if Identity team...
- added a boolean test-user column in their DB
- exposed it via a flag in their IDAPI endpoints (notably for manage the
/auth/redirect
endpoint - see wiki/Identity-Integration) - provided either an alternate sign-in flow or a URL param on the current flow to set this flag on the user
Furthermore, the 48hour thing is fairly pointless since the identity accounts don't get deleted after the 48hours expire, they simply make the systems silently flip back from pointing to UAT Zuora/Salesforce to the normal environment for the Stage.
Currently, there's a mix of the two approaches (typically newer things are hooks-based) but there's probably some gains to made by making them consistently hooks-based functional components. One should proceed with caution though and do some diligent manual regression testing in such refactors as there are some subtle differences in how the lifecycle works (for example setState
has overloads, one of which was used in the card update flow but when migrated to the simpler hooks a race condition was introduced [just in DEV/CODE] - see pull/450#discussion_r448919898)
Not what you're looking for? Be sure to use the navigation sidebar on the right. ➡️