Refactor fetching - Githubissues

Devographics / Monorepo

Monorepo containing the State of JS apps

surveyform-sigma.vercel.app

Other

131 stars 52 forks source link

Refactor fetching #218

Closed SachaG closed 1 year ago

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
surveyform	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 10, 2023 5:50am

netlify[bot] commented 1 year ago

Deploy Preview for stateofjs canceled.

Name	Link
Latest commit	ca430ecfe9553ea929594d002846089d5c4d46c4
Latest deploy log	https://app.netlify.com/sites/stateofjs/deploys/645b2feb168e070008b9c523

netlify[bot] commented 1 year ago

Deploy Preview for stateofjs2022 canceled.

Name	Link
Latest commit	ca430ecfe9553ea929594d002846089d5c4d46c4
Latest deploy log	https://app.netlify.com/sites/stateofjs2022/deploys/645b2feb6a026200082fe2c2

netlify[bot] commented 1 year ago

Deploy Preview for state-of-graphql canceled.

Name	Link
Latest commit	ca430ecfe9553ea929594d002846089d5c4d46c4
Latest deploy log	https://app.netlify.com/sites/state-of-graphql/deploys/645b2febed71f10008723044

netlify[bot] commented 1 year ago

Deploy Preview for tokyodev2022 canceled.

Name	Link
Latest commit	ca430ecfe9553ea929594d002846089d5c4d46c4
Latest deploy log	https://app.netlify.com/sites/tokyodev2022/deploys/645b2feb6a026200082fe2c7

netlify[bot] commented 1 year ago

Deploy Preview for devographics-graphiql2 canceled.

Name	Link
Latest commit	ca430ecfe9553ea929594d002846089d5c4d46c4
Latest deploy log	https://app.netlify.com/sites/devographics-graphiql2/deploys/645b2febf4b7530008e31769

netlify[bot] commented 1 year ago

Deploy Preview for stateofcss canceled.

Name	Link
Latest commit	ca430ecfe9553ea929594d002846089d5c4d46c4
Latest deploy log	https://app.netlify.com/sites/stateofcss/deploys/645b2febed71f10008723049

netlify[bot] commented 1 year ago

Deploy Preview for devographics-graphiql-internal canceled.

Name	Link
Latest commit	ca430ecfe9553ea929594d002846089d5c4d46c4
Latest deploy log	https://app.netlify.com/sites/devographics-graphiql-internal/deploys/645b2feb0962510007e2d5d3

netlify[bot] commented 1 year ago

Deploy Preview for graphiql-devographics canceled.

Name	Link
Latest commit	ca430ecfe9553ea929594d002846089d5c4d46c4
Latest deploy log	https://app.netlify.com/sites/graphiql-devographics/deploys/645b2febc524230008fb0bf9

netlify[bot] commented 1 year ago

Deploy Preview for devographics-static failed.

Name	Link
Latest commit	ca430ecfe9553ea929594d002846089d5c4d46c4
Latest deploy log	https://app.netlify.com/sites/devographics-static/deploys/645b2febed71f1000872303f

gatsby-cloud[bot] commented 1 year ago

:x: State of CSS 2022 deploy preview failed

Your build failed. View the build logs.

gatsby-cloud[bot] commented 1 year ago

:x: State of CSS 2022 deploy preview failed

Your build failed. View the build logs.

gatsby-cloud[bot] commented 1 year ago

:x: State of GraphQL 2022 deploy preview failed

Your build failed. View the build logs.

gatsby-cloud[bot] commented 1 year ago

:x: State of GraphQL 2022 deploy preview failed

Your build failed. View the build logs.

eric-burel commented 1 year ago

@SachaG I've progressed a bit on testing the thing, I've refactored the way we get surveyParamsTable: this is something that should be fetched once for all in the layout that loads the edition, and then either passed via props to RSC or context for client-side. What needs to be done:

I've moved the function that get the surveyParamsTable in "surveys/data" so we can find it easily. In order to avoid hard-writing this table, I've also created a ".json" at the root of the surveys repo => last step is to create an API endpoint that returns this value from GitHub, as we do for the surveys list, I didn't do this step yet
This table is actually only ever needed when we retrieve the edition in the layout when accessing "state-of-js/2022", in order to retrieve the corresponding surveyId and editionId. However, since it's used by helpers like "getEditionSectionPath" too, it ends up being used also in places where we already have the "edition" object available, this is not needed. The rule of thumb should be that if you have only the URL, you need this table, if you have the edition, you don't need this table. My question: do we still have a "slug" parameter or equivalent that would let us keep this info directly within the edition? This would simplify the code a lot (removing lot of useless "editionPathSegments" variables).

There are still a few typings issue eg "shareUrl" that do not exist anymore or similar, that prevents building, but we are progressing.

SachaG commented 1 year ago

do we still have a "slug" parameter or equivalent that would let us keep this info directly within the edition?

Yes but for this to work I think we would need the API to accept that slug parameter as an argument when fetching an edition? We could do that I think as long as we make it clear in the code that the survey slug + year combination is only used in surveyform and that the canonical identifier is still id.

Another option would be to keep surveyParamsTable for surveys up to now as a redirect table, and from now on use state-of-css/css2022, state-of-js/js2023, etc. instead in the URLs. This way:

the actual edition id would be included in the URL and avoid the need for introducing new arguments to the API
if we ever want to do more than one survey a year (e.g. state-of-js/summer2023quiz or whatever) we are not limited by the URL structure

But anyway I think we can just do whatever's simplest for now and think about getting rid of the hardcoded json starting next year.

eric-burel commented 1 year ago

I think we would need the API to accept that slug parameter as an argument when fetching an edition You fetch the edition only once in the layout when you first access this specific edition, in this case the surveyParamsTable is appropriate so we remove the need to pass the slug as argument. But the cases that bothers me is when you already have the edition, and you just want to build some links href, currently we also use the surveyParamTable but it's not really needed, since we already have fetched the data.

I am ok with the current architecture, I think it's clean, but just want to keep things as simple and clean as possible as we go, because cleaning afterwards can be more time-consuming than expected. If you want more than 1 survey in a year you can use a non-number year like "2023-summer-quiz" would work fine and will not clash with "2023" in the table

Edit: to clarify we can call them surveySlug and editionSlug

eric-burel commented 1 year ago

@SachaG do we have an endpoint to get just the metadata for a survey, without editions? When computing the magic link we may want to get the domain of the survey, and the parameter we have is the surveyId, currently I've reused fetchEditionMetadataSurveyForm but it also require an editionId that is not relevant here

eric-burel commented 1 year ago

Almost done with reenabling the build, but I miss hashTag and shareUrl in "Score.tsx", where are they located?

Note that the knowledge score is currently wrong because it doesn't get the full user response but I'll take care of this, I just need to reenable the build so I can run e2e tests safely

SachaG commented 1 year ago

@SachaG do we have an endpoint to get just the metadata for a survey, without editions? When computing the magic link we may want to get the domain of the survey, and the parameter we have is the surveyId, currently I've reused fetchEditionMetadataSurveyForm but it also require an editionId that is not relevant here

I added a new fetchSurveyMetadata function. I did include the editions so that it can be reused for e.g. stateofjs.com to get all the relevant metadata about a survey. That being said we could also just reuse fetchSurveysMetadata, I don't think it's that big a deal if we overfetch the other survey's metadata.