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

[FEATURE] Ajout de la possibilité * comme paramètre (PIX-15594) #59

Merged
merged 2 commits into from
Dec 16, 2024

Conversation

sandalfon
Copy link
Contributor

@sandalfon sandalfon commented Dec 4, 2024

🎄 Problème

aujourd'hui les paramètres doivent avoir leur valeur définie, pour parcoursup il faut requeter sur différents id non connu à l'avance

🎁 Proposition

ajout du * comme valeur wildecard

🧦 Remarques

🚨cela fonctionne pour * donc un string, actuellement rien n'est prévu si le paramètre est un numérique, pas sur que la bd accepte * en valeur

🎅 Pour tester

@pix-bot-github
Copy link

Une fois l'application déployée, elle sera accessible à cette adresse https://api-data-pr59.review.pix.fr
Les variables d'environnement seront accessibles sur scalingo https://dashboard.scalingo.com/apps/osc-fr1/pix-api-data-integration-pr59/environment

@Gudsfile Gudsfile changed the title [FEATURE]: ajout de la possibilité * comme paramètre [FEATURE] Ajout de la possibilité * comme paramètre Dec 5, 2024
@Gudsfile Gudsfile changed the title [FEATURE] Ajout de la possibilité * comme paramètre [FEATURE] Ajout de la possibilité * comme paramètre (PIX-15594) Dec 5, 2024
@sandalfon sandalfon force-pushed the feature-add-wildcard-for-queryparams branch 2 times, most recently from 7088801 to c510662 Compare December 5, 2024 10:39
@sandalfon sandalfon force-pushed the feature-add-wildcard-for-queryparams branch from c510662 to 890df7f Compare December 5, 2024 14:06
@sandalfon sandalfon force-pushed the feature-add-wildcard-for-queryparams branch 2 times, most recently from 69622ad to 488602f Compare December 10, 2024 08:11
@pix-service-auto-merge pix-service-auto-merge force-pushed the feature-add-wildcard-for-queryparams branch from 488602f to 6083e88 Compare December 11, 2024 13:13
Copy link
Contributor

@Steph0 Steph0 left a comment

Choose a reason for hiding this comment

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

Hello,

Serait-il possible de remplir la section a tester afin de verifier fonctionnellement cette PR ? Merci :)

Autre element : je ne comprends pas trop la remarque, j'ai du mal a voir pourquoi pour une valeur numerique cela ne fonctionnerait pas ?

lib/domain/models/QueryAccess.ts Outdated Show resolved Hide resolved
@sandalfon
Copy link
Contributor Author

Hello,

Serait-il possible de remplir la section a tester afin de verifier fonctionnellement cette PR ? Merci :)

Autre element : je ne comprends pas trop la remarque, j'ai du mal a voir pourquoi pour une valeur numerique cela ne fonctionnerait pas ?

Je reformule
On a une étape de vérification du type de paramètre

si on prend le numéro de département en paramètre c'est un entier
si j'envoie une demande avec numéro = any
je vais avoir
violates check constraint "catalog_query_params_type_check"

Comment on lines 296 to 332
it('should return a proper error with status code 403', async function () {
// given
const queryId = '26f6efcc-ce13-4b20-b6ea-5bebae6115af';

await knexAPI('catalog_queries').insert({
id: queryId,
sql_query: `SELECT COUNT(*) as count FROM public.data_ref_academies where nom = {{ academie }}`,
name: 'foo',
});
await knexAPI('query_access').insert({
query_id: queryId,
user_id: userId,
});
await knexAPI('catalog_query_params').insert({
id: 1,
catalog_query_id: queryId,
name: 'academie',
type: 'string',
mandatory: true,
});
await knexAPI('query_param_access').insert({
id: queryId,
user_id: userId,
query_param_id: 1,
value: 'Bordeaux',
});

const payload = {
queryId,
params: <UserCommandParam[]>[{ name: 'academie', value: 'Paris' }],
};

// when
const server = await createServer();
const response = await server.inject({
method: 'POST',
Copy link
Member

Choose a reason for hiding this comment

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

Ce test correspond au test d'avant, on peut le supprimer je pense

@VincentHardouin VincentHardouin force-pushed the feature-add-wildcard-for-queryparams branch from 8824dd9 to ade8efd Compare December 16, 2024 15:04
@pix-service-auto-merge pix-service-auto-merge force-pushed the feature-add-wildcard-for-queryparams branch from ade8efd to 3f8619e Compare December 16, 2024 15:05
@pix-service-auto-merge pix-service-auto-merge merged commit dfbd2d7 into dev Dec 16, 2024
5 of 6 checks passed
@pix-service-auto-merge pix-service-auto-merge deleted the feature-add-wildcard-for-queryparams branch December 16, 2024 15:07
pix-service-auto-merge pushed a commit that referenced this pull request Dec 16, 2024
# [0.11.0](v0.10.2...v0.11.0) (2024-12-16)

### 🚀 Amélioration

- [#59](#59) Ajout de la possibilité * comme paramètre (PIX-15594)

### 🏗️ Tech

- [#63](#63) Proposition de modifications pour le setup de developement
@pix-service-auto-merge
Copy link
Contributor

🎉 This PR is included in version 0.11.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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.

6 participants