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

Choix date signature #1586

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Choix date signature #1586

wants to merge 10 commits into from

Conversation

syldb
Copy link
Contributor

@syldb syldb commented Sep 23, 2024

TODO:

  • Tests
  • Valeur par défaut à aujourd'hui

Copy link

github-actions bot commented Sep 23, 2024

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
8284 6890 83% 80% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
conventions/services/recapitulatif.py 70% 🟢
conventions/urls.py 100% 🟢
conventions/views/conventions.py 63% 🟢
TOTAL 78% 🟢

updated for commit: 5133f0c by action🐍

@syldb syldb force-pushed the choix-date-signature branch 2 times, most recently from 8b2b082 to 1ef6f3e Compare September 26, 2024 06:43
@syldb syldb marked this pull request as ready for review September 26, 2024 06:43
@syldb syldb requested a review from a team as a code owner September 26, 2024 06:43
@syldb syldb requested review from kolok and etchegom and removed request for a team September 26, 2024 06:43
Copy link
Collaborator

@kolok kolok left a comment

Choose a reason for hiding this comment

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

Mini commentaire
J'ai hate de voir la démo :)

@@ -58,6 +59,8 @@
from upload.models import UploadedFile
from upload.services import UploadService

path_to_sent = "conventions/sent.html"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
path_to_sent = "conventions/sent.html"
template_sent = "conventions/sent.html"

?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Mais en vrai, je pense que cet écra ne sert à rien et on devrait revenir au récapitulatif avec une notification du succès de signature : qu'en pense tu ?

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'ai créé cette variable parce que sonarcloud se plaignait
Oui c'est une bonne idée

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah sent c'est la page depuis laquelle on part, elle ressemble à ça :

Capture d’écran 2024-09-26 à 09 25 10

Je redirige vers le post_action à la fin du processus, j'ajoute le message

Copy link
Collaborator

@kolok kolok left a comment

Choose a reason for hiding this comment

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

En local, je n'ai pas vu mon document s'afficher, mais peut-être ma config est pourrie

ça vaut le coup d'utiliser django.messages pour afficher une notification de succès de téléchargement

@syldb
Copy link
Contributor Author

syldb commented Sep 26, 2024

En local selon les docs je les vois ou pas, mais l'upload marche bizarrement en local.
C'est fait pour django messages, j'attends de démo lundi à tout le monde pour merger !

Copy link

sonarcloud bot commented Sep 26, 2024

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