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

[BUGFIX] Ne plus envoyer de 500 lors d'une recherche par ID invalide sur 2 pages PixAdmin (PIX-12576) #8962

Merged

Conversation

Alexandre-Monney
Copy link
Contributor

🦄 Problème

Dans les filtres de recherche par ID de la page "Profil Cibles" onglet Organizations, ainsi que sur la page "Organisations", lorsque l'on saississait une valeur trop elevée (> a la valeur int de postgres) l'api nous renvoyait une 500 car PG ne gère pas les id allant au dela.

🤖 Proposition

Faire une validation JOI des query params d'id, et si ils ne rentrent pas dans la plage prévue par PG l'erreur sera throw par joi et sera donc une 400.

🌈 Remarques

On a fait une validation Joi coté API. Mais le front continuait d'afficher "Erreur dans Ember" car il se retrouvait avec une erreur dans son fetch de données. On a ajouté un try/catch dans deux des pages problématiques, et si une erreur est remontée on retourne un tableau vide.
Il faudrait peut être dans un second temps, afficher une notification de cette erreur et afficher à l'utilisateur la raison du problème.
On en profite pour retirer un console.log qui trainait dans les tests et qui affichait tout un tas d'infos en console.

💯 Pour tester

  • Se connecter à PixAdmin
  • Aller sur la page Profil Cible
  • Selectionner un Profil Cible au hasard
  • Cliquer sur l'onglet Organisations
  • Entrer un nombre trop grand (du style : 49590747098427509842) dans le filtre par ID
  • Vérifier que l'application ne crash pas, et que rien ne s'affiche
  • Refaire la même manip dans la page "Organisations" avec le filtre par ID également
  • 🐈‍⬛

@pix-bot-github
Copy link

Une fois les applications déployées, elles seront accessibles via les liens suivants :

Les variables d'environnement seront accessibles via les liens suivants :

@@ -50,4 +51,21 @@ module('Unit | Route | authenticated/target-profiles/target-profile/organization
});
});
});

module('#model', function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@@ -66,6 +66,21 @@ module('Unit | Route | authenticated/organizations/list', function (hooks) {
assert.ok(true);
});
});

module('when an error occurs', function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

🎆

@@ -80,7 +80,6 @@ function _findByListItemText(screen, text) {
return (
screen.getAllByRole('listitem').find((listitem) => {
const cleanListItemText = listitem.textContent.replace(/(\r\n|\n|\r)/gm, '').trim();
console.log(cleanListItemText);
Copy link
Contributor

Choose a reason for hiding this comment

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

😅 , il est rigolo lui

@xav-car
Copy link
Contributor

xav-car commented May 17, 2024

✨Func ok ✨

@lionelB lionelB added Func Review OK PO validated functionally the PR and removed 👀 Func Review Needed labels May 21, 2024
@Alexandre-Monney Alexandre-Monney force-pushed the pix-12576-fix-500-with-big-int-search-pix-admin branch from 6d41684 to d5646ed Compare May 21, 2024 16:16
@Alexandre-Monney Alexandre-Monney force-pushed the pix-12576-fix-500-with-big-int-search-pix-admin branch from d5646ed to 36560f5 Compare May 21, 2024 16:32
@pix-service-auto-merge pix-service-auto-merge merged commit eeedc37 into dev May 21, 2024
7 checks passed
@pix-service-auto-merge pix-service-auto-merge deleted the pix-12576-fix-500-with-big-int-search-pix-admin branch May 21, 2024 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants