feat: 1406 contrib affichage des status sur ladmin #1425

Viczei · 2024-06-06T13:52:06Z

No description provided.

github-advanced-security · 2024-06-06T14:46:27Z

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

maxgfr · 2024-06-12T09:06:44Z

targets/frontend/src/modules/contribution/__tests__/mapContributionToDocument.test.ts

@@ -24,6 +24,7 @@ describe("mapContributionToDocument", () => {
  };

  it("devrait mapper l'answer d'un document sans fiche SP", async () => {
+    // @ts-ignore


Pourquoi un ts-ignore, tu aurais pu mocker directement les valeurs, ça aurait été plus clean

Et ça permet en cas de modification des données d'avoir un retour plus rapide sur ces tests. Après c'est vraiment galère des jeux de données aussi gros à maintenir :(

maxgfr · 2024-06-12T09:07:25Z

targets/hasura/metadata/databases/default/tables/model_models.yaml

-        - intro
+        - description


Pourquoi on le modifie dans cette PR ça

ah non ca c'est une erreur. Je comprends pas pourquoi mon hasura remet toujours ce champs description alors que je l'ai dégagé

maxgfr · 2024-06-12T09:09:02Z

targets/hasura/metadata/databases/default/tables/public_documents.yaml

+    export_id:
+      custom_name: exportId
+  custom_column_names:
+    export_id: exportId


Mais du coup, tous les documents auront le même exportId, je vois pas comment c'est possible que les deux soient différents

Pour le coup la gymnastique n'est pas la plus clean peut être mais au moment de l'export, on détecte les documents qui ont changé depuis la dernière mise à jour des données. Puis on va mettre à jour l'export id seulement pour ces documents. Cela ne concerne que les contributions pour le moment d'ailleurs.

Oui j'initialise dans mon script de migration tous les documents sur le dernier export mais ensuite ils sont maj indépendamment au niveau de l'export

maxgfr · 2024-06-12T09:09:52Z

targets/frontend/src/components/contributions/status/StatusPublication.tsx

+  let tooltipText: string | undefined;
+  if (!exportDate) {
+    status = "NOT_PUBLISHED";
+    tooltipText = statusesMapping[status].text;
+  } else {
+    status = "PUBLISHED";
+    tooltipText = `${statusesMapping[status].text} le ${format(
+      parseISO(exportDate),
+      "dd/MM/yyyy HH:mm:ss"
+    )}`;
+  }


On aurait pu le mettre dans un useEffect, ça aurait opti un peu plus le code car là ça va se trigger à chaque fois

C'est un écran de visualisation sans édition, il n'y a pas de modification de ces données donc pas de refresh.

maxgfr · 2024-06-12T09:10:04Z

targets/frontend/src/components/contributions/status/Status.tsx

+  if (status === "TO_PUBLISH") {
+    if (statusDate && exportDate && isPublished({ statusDate, exportDate })) {
+      status = "TO_PUBLISH";
+    } else {
+      status = "PUBLISHING";
+    }
+  }
+  const tooltipTitle = statusesMapping[status].text;


maxgfr · 2024-06-12T09:12:23Z

targets/export-elasticsearch/src/ingester/cdtnDocuments.ts

@@ -66,6 +71,8 @@ export async function cdtnDocumentsGen(
  const glossaryTerms = await getGlossary();
  const addGlossary = createGlossaryTransform(glossaryTerms);

+  const contributionsToPublish = await fetchContributionDocumentToPublish();


C'est dommage qu'on soit obligé de fetch deux fois les contributions, on aurait pu éviter cela en vérifiant une propriété des contributions, non ?

On ne fetch pas toutes les contribs là je crois, on ne récupère que celle qui ont été modifiées : https://github.com/SocialGouv/cdtn-admin/pull/1425/files#diff-046031753264baf1637cef36c0bc23cd316e32a73e7250a81133410e2c0eed4fR8

Je pense que le nommage peut être amélioré ici pour éviter cette confusion :

Suggested change

const contributionsToPublish = await fetchContributionDocumentToPublish();

const editedContributionsSinceLastPublish = await fetchEditedContributionDocument();

maxgfr · 2024-06-12T09:14:04Z

targets/export-elasticsearch/src/ingester/ingest.ts

@@ -89,7 +92,8 @@ async function runIngester(
  suggestIndexName: string | undefined,
  bufferSize: number | undefined,
  suggestFile: string | undefined,
-  disableGlossary: boolean | undefined
+  disableGlossary: boolean | undefined,
+  isProd: boolean


Pour toutes ces variables, on utilise des contexte globaux pour éviter de faire du props forwarding, c'est dommage de changer cette approche

ça revient à deux approches différentes dans le même code

Faudra que tu me montre, j'avais compris qu'il fallait faire avec du props forwarding de mon côté

m-maillot

Quelques remarques mais pas bloquants pour une mise en prod pour moi.

m-maillot · 2024-06-12T15:35:33Z

targets/export-elasticsearch/src/ingester/cdtnDocuments.ts

@@ -66,6 +71,8 @@ export async function cdtnDocumentsGen(
  const glossaryTerms = await getGlossary();
  const addGlossary = createGlossaryTransform(glossaryTerms);

+  const contributionsToPublish = await fetchContributionDocumentToPublish();


On ne fetch pas toutes les contribs là je crois, on ne récupère que celle qui ont été modifiées : https://github.com/SocialGouv/cdtn-admin/pull/1425/files#diff-046031753264baf1637cef36c0bc23cd316e32a73e7250a81133410e2c0eed4fR8

Je pense que le nommage peut être amélioré ici pour éviter cette confusion :

Suggested change

const contributionsToPublish = await fetchContributionDocumentToPublish();

const editedContributionsSinceLastPublish = await fetchEditedContributionDocument();

m-maillot · 2024-06-12T15:42:46Z

targets/frontend/src/components/contributions/answers/Answer.tsx

@@ -71,7 +71,7 @@ export const ContributionsAnswer = ({
        cdtnReferences: data.cdtnReferences,
        otherReferences: data.otherReferences,
      });
-      if (newStatus === "PUBLISHED") {
+      if (newStatus === "TO_PUBLISH") {


Idée de nommage : READY

Le TO_PUBLISH est confusant car il va rester même après une mise à jour des données. Le status READY indique qu'elle est prête pour la prod. Une fois en prod, elle reste READY qui ne porte pas à confusion comme le TO_PUBLISH.

Je crois que j'aurai laissé "PUBLISHED" qui correspond à ce qu'on affiche su l'ui.

Un des retours du métier c'était justement que ce status "PUBLISHED" n'était pas claire parce que ca prête à confusion vu que le document n'est pas encore en prod. J'aime bien l'idée de le nommé "READY" (ou "prêt" en francais)

c'est pour ça que dans le court moment où la contrib est publié mais pas encore mis en prod on affiche "à publier" et on la mets en orange. Mais le reste du temps elle est bien publié

m-maillot · 2024-06-12T15:48:20Z

targets/frontend/src/components/contributions/status/StatusPublication.tsx

+  let tooltipText: string | undefined;
+  if (!exportDate) {
+    status = "NOT_PUBLISHED";
+    tooltipText = statusesMapping[status].text;
+  } else {
+    status = "PUBLISHED";
+    tooltipText = `${statusesMapping[status].text} le ${format(
+      parseISO(exportDate),
+      "dd/MM/yyyy HH:mm:ss"
+    )}`;
+  }


C'est un écran de visualisation sans édition, il n'y a pas de modification de ces données donc pas de refresh.

m-maillot · 2024-06-12T15:51:03Z

targets/frontend/src/modules/contribution/__tests__/mapContributionToDocument.test.ts

@@ -24,6 +24,7 @@ describe("mapContributionToDocument", () => {
  };

  it("devrait mapper l'answer d'un document sans fiche SP", async () => {
+    // @ts-ignore


Et ça permet en cas de modification des données d'avoir un retour plus rapide sur ces tests. Après c'est vraiment galère des jeux de données aussi gros à maintenir :(

m-maillot · 2024-06-12T15:52:55Z

targets/hasura/metadata/databases/default/tables/public_documents.yaml

+    export_id:
+      custom_name: exportId
+  custom_column_names:
+    export_id: exportId


Pour le coup la gymnastique n'est pas la plus clean peut être mais au moment de l'export, on détecte les documents qui ont changé depuis la dernière mise à jour des données. Puis on va mettre à jour l'export id seulement pour ces documents. Cela ne concerne que les contributions pour le moment d'ailleurs.

m-maillot · 2024-06-12T15:55:45Z

targets/hasura/migrations/default/1718022780364_set_export_id_documents_contrib/up.sql

+update public.documents d
+set export_id = le.id
+from _last_export le
+where (d.source = 'contributions' or d.source = 'information' or d.source = 'modeles_de_courriers')


on ne le fait pas que sur les contributions ? Car actuellement, on ne gère que ces documents là : https://github.com/SocialGouv/cdtn-admin/pull/1425/files#diff-046031753264baf1637cef36c0bc23cd316e32a73e7250a81133410e2c0eed4fR31
Autant laisser à NULL dans la table pour le moment (car pas fiable vu que pas mis à jour).

ok oui je peux faire que les contribs pour le moment

carolineBda · 2024-06-20T12:39:44Z

targets/frontend/src/components/contributions/answers/Comment.tsx

@@ -81,7 +82,9 @@ export const Comment = ({ comment, onDelete }: Props) => {
        }}
      >
        {" "}
-        {statusesMapping[comment.status].text}
+        {statusesMapping[comment.status].text === "Publié"


En fait ici, on peut laisser "Publié" dans les commentaires, comme ça c'est cohérent partout

sonarqubecloud · 2024-06-20T13:41:23Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

github-actions · 2024-06-20T13:50:01Z

🎉 Deployment for commit c554821 :

Ingresses

🚀 https://www-cdtn-admin-1406-contrib-affichage-des-status-sur-ladmin.ovh.fabrique.social.gouv.fr/

Docker images

📦 docker pull harbor.fabrique.social.gouv.fr/cdtn/cdtn-admin/export:sha-c55482127cd2065f9dcedf25270e56cef4ac3f31
📦 docker pull harbor.fabrique.social.gouv.fr/cdtn/cdtn-admin/frontend:sha-c55482127cd2065f9dcedf25270e56cef4ac3f31
📦 docker pull harbor.fabrique.social.gouv.fr/cdtn/cdtn-admin/hasura:sha-c55482127cd2065f9dcedf25270e56cef4ac3f31

Debug

victor added 3 commits June 6, 2024 09:01

feat: set publish information to contriibutions

f81e68f

feat: implement export update status

f4cdea4

chore: update TU

735c7bb

Viczei requested review from m-maillot, carolineBda and maxgfr June 6, 2024 13:52

Viczei self-assigned this Jun 6, 2024

Viczei linked an issue Jun 6, 2024 that may be closed by this pull request

[Contrib] Affichage des status sur l'admin #1406

Closed

6 tasks

Viczei changed the title ~~1406 contrib affichage des status sur ladmin~~ feat: 1406 contrib affichage des status sur ladmin Jun 6, 2024

chore: eslint

74b922f

Viczei had a problem deploying to review-auto June 6, 2024 14:16 — with GitHub Actions Failure

chore: clean

cbd9751

Viczei had a problem deploying to review-auto June 6, 2024 14:30 — with GitHub Actions Error

fix: mistype

d6e391c

Viczei had a problem deploying to review-auto June 6, 2024 14:32 — with GitHub Actions Error

chore: clean

40bfa47

Viczei temporarily deployed to review-auto June 6, 2024 14:33 — with GitHub Actions Inactive

chore: clean

ab7e46e

Viczei temporarily deployed to review-auto June 6, 2024 15:27 — with GitHub Actions Inactive

chore: fix optional type

aa99502

Viczei had a problem deploying to review-auto June 7, 2024 07:46 — with GitHub Actions Failure

Viczei temporarily deployed to review-auto June 7, 2024 08:26 — with GitHub Actions Inactive

refactor: review

8ee2b9c

Viczei had a problem deploying to review-auto June 10, 2024 10:18 — with GitHub Actions Failure

fix: publishing status not displaying

053b906

Viczei had a problem deploying to review-auto June 10, 2024 11:19 — with GitHub Actions Failure

fix: build

101c285

Viczei had a problem deploying to review-auto June 10, 2024 11:30 — with GitHub Actions Error

feat: add migration

6c28b9c

Viczei temporarily deployed to review-auto June 10, 2024 11:43 — with GitHub Actions Inactive

feat: script migration update export_id

800c8d1

Viczei temporarily deployed to review-auto June 10, 2024 12:33 — with GitHub Actions Inactive

feat: add migratrion script relation contrib/doc

e269a0f

Viczei temporarily deployed to review-auto June 10, 2024 13:18 — with GitHub Actions Inactive

fix: maj not running

7356762

Viczei temporarily deployed to review-auto June 10, 2024 14:22 — with GitHub Actions Inactive

victor added 2 commits June 10, 2024 18:12

Merge branch 'master' into 1406-contrib-affichage-des-status-sur-ladmin

2cc2dda

refactor: review

18eee26

Viczei had a problem deploying to review-auto June 11, 2024 15:11 — with GitHub Actions Failure

maxgfr reviewed Jun 12, 2024

View reviewed changes

m-maillot approved these changes Jun 12, 2024

View reviewed changes

Viczei temporarily deployed to review-auto June 13, 2024 07:33 — with GitHub Actions Inactive

chore: clean

0837d9f

Viczei had a problem deploying to review-auto June 14, 2024 14:37 — with GitHub Actions Error

chore: clean

0e4f7cf

Viczei temporarily deployed to review-auto June 14, 2024 14:38 — with GitHub Actions Inactive

fix: à publier sur page réponse

0a65902

Viczei temporarily deployed to review-auto June 17, 2024 08:19 — with GitHub Actions Inactive

carolineBda reviewed Jun 20, 2024

View reviewed changes

victor added 2 commits June 20, 2024 14:50

Merge branch 'master' into 1406-contrib-affichage-des-status-sur-ladmin

641e499

chore: review

2b9da39

Viczei temporarily deployed to review-auto June 20, 2024 13:14 — with GitHub Actions Inactive

chore: review

c554821

Viczei temporarily deployed to review-auto June 20, 2024 13:40 — with GitHub Actions Inactive

Viczei merged commit 9f63a2a into master Jun 20, 2024
29 checks passed

Viczei deleted the 1406-contrib-affichage-des-status-sur-ladmin branch June 20, 2024 14:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: 1406 contrib affichage des status sur ladmin #1425

feat: 1406 contrib affichage des status sur ladmin #1425

Viczei commented Jun 6, 2024

github-advanced-security bot commented Jun 6, 2024

maxgfr Jun 12, 2024

m-maillot Jun 12, 2024

maxgfr Jun 12, 2024

Viczei Jun 13, 2024

maxgfr Jun 12, 2024

m-maillot Jun 12, 2024

Viczei Jun 13, 2024

maxgfr Jun 12, 2024

m-maillot Jun 12, 2024

maxgfr Jun 12, 2024

maxgfr Jun 12, 2024

m-maillot Jun 12, 2024

maxgfr Jun 12, 2024

maxgfr Jun 12, 2024

Viczei Jun 13, 2024

m-maillot left a comment

m-maillot Jun 12, 2024

m-maillot Jun 12, 2024

carolineBda Jun 13, 2024

Viczei Jun 13, 2024

carolineBda Jun 20, 2024

m-maillot Jun 12, 2024

m-maillot Jun 12, 2024

m-maillot Jun 12, 2024

m-maillot Jun 12, 2024

Viczei Jun 13, 2024

carolineBda Jun 20, 2024

sonarqubecloud bot commented Jun 20, 2024

github-actions bot commented Jun 20, 2024

	const contributionsToPublish = await fetchContributionDocumentToPublish();
	const editedContributionsSinceLastPublish = await fetchEditedContributionDocument();

feat: 1406 contrib affichage des status sur ladmin #1425

feat: 1406 contrib affichage des status sur ladmin #1425

Conversation

Viczei commented Jun 6, 2024

github-advanced-security bot commented Jun 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m-maillot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Jun 20, 2024

Quality Gate passed

github-actions bot commented Jun 20, 2024