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

notify convention need signature after been modified #263

Conversation

bbohec
Copy link
Contributor

@bbohec bbohec commented Jun 7, 2023

No description provided.

@bbohec bbohec marked this pull request as draft June 7, 2023 10:03
@bbohec bbohec changed the title WIP notify convention need signature after been modified Jun 7, 2023
@bbohec bbohec self-assigned this Jun 7, 2023
@bbohec bbohec force-pushed the 174-etq-utilisateur-et-acteur-dune-demande-de-convention-je-reçois-un-mail-spécifique-pour-minformer-que-la-demande-de-convention-a-été-modifiée-par-un-autre-acteur-et-que-je-dois-la-signer-de-nouveau branch 6 times, most recently from 76918ff to 40283ad Compare June 14, 2023 12:01
@bbohec
Copy link
Contributor Author

bbohec commented Jun 14, 2023

Make sure that statusJustification is kept on convention modification phase in front.

image

@bbohec bbohec force-pushed the 174-etq-utilisateur-et-acteur-dune-demande-de-convention-je-reçois-un-mail-spécifique-pour-minformer-que-la-demande-de-convention-a-été-modifiée-par-un-autre-acteur-et-que-je-dois-la-signer-de-nouveau branch from 2a74b48 to 283f993 Compare June 14, 2023 14:16
@bbohec
Copy link
Contributor Author

bbohec commented Jun 14, 2023

Email sample
image

@bbohec bbohec marked this pull request as ready for review June 14, 2023 14:24
Comment on lines 49 to 52
if (req.payloads?.backOffice || req.payloads?.convention) {
return deps.useCases.updateConvention.execute(req.body);
}

if (!req.payloads?.convention) throw new UnauthorizedError();
return deps.useCases.updateConvention.execute({
id: req.payloads.convention.applicationId,
convention: req.body,
});
throw new UnauthorizedError();
Copy link
Contributor

Choose a reason for hiding this comment

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

ici c'est un cas où je trouve le if en guard bien plus clair , comme ceci :

        // check de droits
        if (!req.payloads?.backOffice && !req.payloads?.convention)
            throw new UnauthorizedError();
            
        // usecase si tu as le droit
        return deps.useCases.updateConvention.execute(req.body);

Comment on lines +67 to +74
const statusJustification =
params.status === "CANCELLED" ||
params.status === "REJECTED" ||
params.status === "DRAFT" ||
params.status === "DEPRECATED"
? params.statusJustification
: undefined;

Copy link
Contributor

Choose a reason for hiding this comment

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

tu as des utilitaires qui peuvent t'aider dans convention.dto.ts

Genre doesStatusNeedsJustification, ou tout simplement :

Suggested change
const statusJustification =
params.status === "CANCELLED" ||
params.status === "REJECTED" ||
params.status === "DRAFT" ||
params.status === "DEPRECATED"
? params.statusJustification
: undefined;
const statusJustification = conventionStatusesWithJustification.includes(params.status)
? params.statusJustification
: undefined;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

J'avais essayé mais ça ne passe pas niveau TS. Surement un problème de scope sur le status qui ne s'applique pas sur le reste de la convention.

image

Copy link
Contributor

Choose a reason for hiding this comment

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

essaye doesStatusNeedsJustification je crois bien qu'il y a un type guard

Copy link
Contributor

Choose a reason for hiding this comment

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

image


updateStatus$(
newConvention$(conventionDto: ConventionDto): Observable<void>;
Copy link
Contributor

Choose a reason for hiding this comment

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

on peut garder le verbe stp: createNewConvention$ ou addNewConvention$

Copy link
Contributor Author

Choose a reason for hiding this comment

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

createConvention$

@bbohec bbohec force-pushed the 174-etq-utilisateur-et-acteur-dune-demande-de-convention-je-reçois-un-mail-spécifique-pour-minformer-que-la-demande-de-convention-a-été-modifiée-par-un-autre-acteur-et-que-je-dois-la-signer-de-nouveau branch from bc94195 to ae3ade8 Compare June 15, 2023 13:11
@bbohec bbohec requested a review from JeromeBu June 15, 2023 13:13
@bbohec bbohec force-pushed the 174-etq-utilisateur-et-acteur-dune-demande-de-convention-je-reçois-un-mail-spécifique-pour-minformer-que-la-demande-de-convention-a-été-modifiée-par-un-autre-acteur-et-que-je-dois-la-signer-de-nouveau branch from ae3ade8 to b1f103a Compare June 15, 2023 14:44
@bbohec bbohec enabled auto-merge (rebase) June 15, 2023 14:45
@bbohec bbohec merged commit fe24763 into dev Jun 15, 2023
@bbohec bbohec deleted the 174-etq-utilisateur-et-acteur-dune-demande-de-convention-je-reçois-un-mail-spécifique-pour-minformer-que-la-demande-de-convention-a-été-modifiée-par-un-autre-acteur-et-que-je-dois-la-signer-de-nouveau branch June 15, 2023 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants