Skip to content

Major TODOs

Tom Richards edited this page Jun 15, 2021 · 58 revisions

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.

Sentry audit

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).

display one-off contributions (mention members-data-api changes required)

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 the fulfilment-date-calculator integration into members-data-api

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).

Replace replica of dotcom footer with a design systems component

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 🙂 ).

Move prettier config to a config file (for consistency)

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.

Delivery Address overhaul

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.

payment method consolidation and switching

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)

critical paths alarms (e.g. no payment updates in the last hour)

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).

'Docker'ise everything

TODO

payment method re-use for subs

TODO (https://github.com/guardian/support-frontend/wiki/Payment-Method-Re-use)

GW renewals (proper)

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

json validation against TS types

TODO

security audit

TODO (notably content security policy & CSRF)

Robots.txt

TODO (could do this via Fastly)

<noscript> tag (since manage is all JS)

TODO

unsupported browser banner (mention previous attempt)

TODO

bundle size review (purge things, plus code splitting and lazy loading)

TODO

Increase test coverage (and move to more genuine coverage rather than snapshot testing)

TODO

overhaul test-users across RR (to become Identity feature)

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.

Replace all React class-based components with hooks-based functional components

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)

Clone this wiki locally