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

Add all request for list features but in a rough state #46

Merged
merged 8 commits into from
Jul 5, 2023
Merged

Conversation

Roukii
Copy link
Contributor

@Roukii Roukii commented Jun 23, 2023

Added all backend request necessary for the lists and list value CRUD
Added dummy button to check the request and interact with the feature

@Roukii Roukii requested a review from balzdur June 23, 2023 14:04
Copy link
Collaborator

@balzdur balzdur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bon travail 👍🏽

libs/api/marble/src/scripts/openapi.yaml Outdated Show resolved Hide resolved
libs/api/marble/src/scripts/openapi.yaml Outdated Show resolved Hide resolved
libs/api/marble/src/lib/generated/marble-api.ts Outdated Show resolved Hide resolved
libs/api/marble/src/lib/generated/marble-api.ts Outdated Show resolved Hide resolved
apps/builder/app/routes/__builder/lists/$listId.tsx Outdated Show resolved Hide resolved
apps/builder/app/routes/__builder/lists/$listId.tsx Outdated Show resolved Hide resolved
@@ -130,9 +162,10 @@ function ScenariosList({ scenarios }: { scenarios: string[] }) {
}

export default function Lists() {
const data = useLoaderData<typeof loader>();
const customListValues = useLoaderData<typeof loader>();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ne conserve pas ce que j'ai fait avant: n'utilise pas const data = ... mais directement listsValues. Ce sera plus lisible

Suggested change
const customListValues = useLoaderData<typeof loader>();
const { listsValues } = useLoaderData<typeof loader>();

accessorFn: (row) => row,
header: t('lists:description'),
accessorKey: 'value',
header: t('lists:value'),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Si tu fais ça, chaud pour que tu updates les translation files (on a pas de CRM/outil externe, ils sont déclarés et maintenu dans le front)

Tu trouveras ce que tu veux ici :
apps/builder/public/locales/en/lists.json (et le fr associé)

Chaud pour que tu supprimes ceux qui ne sont plus utilisés et ajoutent les nouveaux. T'embête pas avec la trad FR.

Comment on lines 46 to 67
export async function action({ request, params }: ActionArgs) {
const { apiClient } = await authenticator.isAuthenticated(request, {
failureRedirect: '/login',
});
console.log(request.method)
invariant(params.listId, `params.listId is required`);
console.log('listId', params.listId)
switch (request.method) {
case 'POST': {
await apiClient.createCustomListValue(params.listId, {value: 'Added Value'});
break;
}
case 'DELETE': {
const parsedForm = await parseFormSafe(request, formSchema);
const { listValueId } = parsedForm.data;
console.log('delete id: ', listValueId)
await apiClient.deleteCustomListValue(params.listId, {id: listValueId});
break;
}
}
return null;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plusieurs remarques :

  • Si tu discrimines sur la method, pas besoin d'ajouter un name au bouton de DELETE
  • Si tu ne fais pas de try ... catch pour les erreurs d'API, tu vas devoir gérer les erreurs en Remix

Dans le cadre d'une erreur non catch, tu peux la gérer avec ça mais sache que dans ce cas précis, toute ta page sera remplacée (ici la page de list view décrite par le composant default export)

apps/builder/app/routes/__builder/lists/index.tsx Outdated Show resolved Hide resolved
@balzdur balzdur merged commit d397a13 into main Jul 5, 2023
1 check passed
@balzdur balzdur deleted the feature/list branch July 5, 2023 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants