Skip to content

Major TODOs

Tom Richards edited this page Jul 6, 2020 · 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]...

Replace custom 'consent banner' with that from CMP library

Currently (see wiki/Cookie-Consent-Banner) we have a custom implementation of the basic 'consent banner' (as all sub-domains did at the time) however things have moved on and much more is required of the consent banner. Fortunately the wonderful @ripecosta and @GHaberis developed a re-usable npm package for the consent banner (@guardian/consent-management-platform) which is already being adopted by other sub-domains (e.g. support-frontend/pull/2569).

The same ought to be done for manage - please update wiki/Cookie-Consent-Banner once done. Notable things...

  • it's unlikely many people see the consent banner on manage as they will have 'accepted' on the main website already - this could be proved by looking at the impression tracking (Ophan only - search for trackEventInOphanOnly in consentsBanner.tsx - also see Ophan/GA tracking)
  • currently the consent banner does not display for the payment flows (achieved using an additional Router in the client-side routing) to avoid drop-off on our most critical journey (especially as some users will be coming to the payment flows via payment failure magic link). This decision probably needs to be revisited to ensure compliance with necessary regulations.
  • The Ophan/GA tracking will need to be wrapped in the consent check before being added to the page etc.

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.

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.

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)

TODO https://prettier.io/docs/en/configuration.html

Delivery Address overhaul

TODO

payment method switching (by consolidating payment methods, like the existing-payment-options endpoint, and offering a tick list of subs to change)

TODO

alarms based on metrics dashboard (e.g. no payment updates in the last hour) - see Server-side Logging & Metrics

TODO

payment method re-use for subs

TODO

GW renewals (proper)

TODO

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

<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