feat: add talking questionnaires synchro #246
Conversation
QRuhier
force-pushed
the
feat/add-talking-surveys
branch
from
2de8b76 to
7d30c90
Compare
QRuhier
force-pushed
the
feat/add-talking-questionnaires-synchro
branch
from
0ace5cb to
4f165e5
Compare
QRuhier
force-pushed
the
feat/add-talking-surveys
branch
from
49f4825 to
c65c4a3
Compare
QRuhier
force-pushed
the
feat/add-talking-questionnaires-synchro
branch
from
6d32848 to
69397b9
Compare
|
| @@ -40,6 +40,22 @@ const surveyProgress = createSelector(downloadingState, (state) => { | |||
| return (state.surveyCompleted * 100) / state.totalSurvey | |||
| }) | |||
|
|
|||
| const externalResourcesProgress = createSelector(downloadingState, (state) => { | |||
| if (state === undefined) { | |||
There was a problem hiding this comment.
You can simplify this into:
if (state === undefined || state.totalExternalResources === undefined) return undefined
| if (state.totalExternalResources === undefined) { | ||
| return undefined | ||
| } | ||
| if ( |
There was a problem hiding this comment.
You can only check state.totalExternalResources === 0
| @@ -54,6 +57,11 @@ export const { reducer, actions } = createUsecaseActions({ | |||
| nomenclatureCompleted: 0, | |||
| totalSurvey: Infinity, | |||
| surveyCompleted: 0, | |||
| // for total external resources, we make difference for displaying progress bar between : | |||
There was a problem hiding this comment.
Couldn't we create a custom state variable to simplify this? e.g. isExternalSynchroTriggered or something that determine whether or not we display the progress bar. This comment seems to imply it is unnecessary complicated as is and totalExternalResources carries two different informations, which it shouldn't.
| if ( | ||
| error instanceof AxiosError && | ||
| error.response && | ||
| [400, 403, 404, 500].includes(error.response.status) |
There was a problem hiding this comment.
This check is used at more than 3 places. The good practice is to make it a reusable function (either in a utils file or in this file if it is only used here).
| if (isNeeded) { | ||
| result.neededQuestionnaires.push(questionnaire) | ||
| } | ||
| // If it's not needed |
There was a problem hiding this comment.
unnecessary comment that only decrease readability by putting else two lines under its if.
|
|
||
| // If the external questionnaire is needed | ||
| if (isNeeded) { | ||
| result.neededQuestionnaires.push(questionnaire) |
There was a problem hiding this comment.
You should make a copy of result and not alter it directly. I think it's bad practice and can create bug here.
There was a problem hiding this comment.
Ok you should not make a copy as it would decrease performance and reduce is (one of) the only case where immutability is thrown off the window BUT instead maybe you should use a for loop instead of reduce to increase performance and readability. (see mdn documentation on reduce method and various benchmarks)
* build: add env var for talking surveys * feat(#240): add capmi app to the router rules * feat(#242): inject externalResources script at start * feat(#238): add global variable before initalize with Lunatic fix: #238 * fix: types in LunaticData * fix: types again We use only one function to initalize surveyUnit correctly Co-Authored-By: Quentin Ruhier <99256289+QRuhier@users.noreply.github.com> * feat(#231): add external service-worker in condition where VITE_EXTERNAL_RESOURCES_URL is defined (#252) fix: #231 * feat: add talking questionnaires synchro (#246) * feat: get externalResources during synchronization * chore: add comments & remove unused imports/exports * fix: get external resources only for needed questionnaires * feat: delete external resources for not needed questionnaires * feat: delete external resources root-cache if no external questionnaire needed * feat: handle progress bar for external resources synchro * fix: handle promises for external resources * perf: optimize cache lookup during external resources synchronization * feat : handle errors on fetching external url * chore: remove unused functions & imports * feat: delete old external resources caches * docs: add external resources synchronization * refactor: simplify externalResources functions * test: set vitest * test: add tests for external resources synchro * fix: import const * ci: add test in jobs * ci: remove condition on target branch for pull_request * ci: fix test job * test : fix sonar config * docs : update external resources synchro * docs: improve external resources synchro * refactor: remove multiple imports + uppercase constants (#257) * refactor : set const for imports of env var VITE_EXTERNAL_RESOURCES_URL * refactor: uppercase constants * refactor: regroup core exported constants * chore: remove unused import * fix: duplicate export * test(sonar) : fix coverage config * ci : adapt CI for tests & sonar * ci: rename ci & jobs --------- Co-authored-by: Quentin Ruhier <quentin.ruhier@insee.fr> Co-authored-by: Quentin Ruhier <99256289+QRuhier@users.noreply.github.com> Co-authored-by: Emmanuel DEMEY <demey.emmanuel@gmail.com>
Handle synchronization step for external resources (talking questionnaires) :