Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

POC an idea for better data fetching, processing and error handling on the page route level #7129

Open
1 task
jroebu14 opened this issue Jul 3, 2020 · 1 comment
Labels
cross-team For visibility for both World Service teams (Engage & Media) Refinement Needed This is ready for refinement. It shouldn't be worked on until it has been refined by Dev & Test. technical-work Technical debt, support work and building new technical tools and features

Comments

@jroebu14
Copy link

jroebu14 commented Jul 3, 2020

Is your feature request related to a problem? Please describe.
An idea was discussed to change the way we fetch and process data and handle errors at the page route level. We should discuss and POC this idea and decide if it's something worth doing.

Describe the solution you'd like
Based on a discussion in this PR:
here #6852 (comment)
and here #6852 (comment)

A proposed implementation might look like

The implementation might look like

const getInitialData = async (urls, mapPageDataToProps) => {
  try {
    const pageData = await Promise.all(fetch(urls));

    return mapPageDataToProps(...pageData);
  } catch ({ message, status }) {
    return { error: message, status };
  }
};

const pageData = await getInitialData(
  ['url-1', 'url-2'],
  (pageData1, pageData2) => ({
    id: pageData1.id,
    mostReadItems: pageData2.mostRead,
  }),
);

we should create a POC with 1 or 2 routes using this idea.

Not all routes have to use this implementation if they have very custom data fetching and processing needs.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Testing notes
[Tester to complete]

Dev insight: Will Cypress tests be required or are unit tests sufficient? Will there be any potential regression? etc

  • This feature is expected to need manual testing.

Additional context
Add any other context or screenshots about the feature request here.

@jroebu14 jroebu14 added Refinement Needed This is ready for refinement. It shouldn't be worked on until it has been refined by Dev & Test. tech-debt technical-work Technical debt, support work and building new technical tools and features labels Jul 3, 2020
@karinathomasbbc karinathomasbbc added cross-team For visibility for both World Service teams (Engage & Media) and removed tech-debt labels Jul 21, 2020
@andrewscfc
Copy link
Contributor

Might be nice to discuss this again, most of the data fetching/error handling is centralised in fetchPageData to my mind. What more does having a standardised getInitialData give us?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cross-team For visibility for both World Service teams (Engage & Media) Refinement Needed This is ready for refinement. It shouldn't be worked on until it has been refined by Dev & Test. technical-work Technical debt, support work and building new technical tools and features
Projects
None yet
Development

No branches or pull requests

3 participants