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

Multi activity workflow #207

Closed
wants to merge 7 commits into from

Conversation

gdaviet
Copy link
Contributor

@gdaviet gdaviet commented Apr 20, 2020

Workflow multi-activités, histoire de compléter la PR précédente.
Du coup @jnguiot finalement j'ai suivi ta suggestion et plus rien n'est sauvegardé tant que le formulaire n'est pas validé avec le bouton du bas.

Cette PR est plutôt pour la validation fonctionnelle, il manque:

  • doc (j'attends d'avoir finalisé l'architecture)
  • Traduction et peaufinage du widget "choices.js" pour le multi-select

@gdaviet gdaviet mentioned this pull request Apr 20, 2020
@gdaviet gdaviet requested a review from jnguiot April 20, 2020 12:25
@gdaviet
Copy link
Contributor Author

gdaviet commented Apr 20, 2020

Note: Il semble y avoir un bug dans la dernière version de sphinx qui est incompatible avec flask: sphinx-doc/sphinx#7516

@jnguiot
Copy link
Member

jnguiot commented Apr 20, 2020

Lol dur... On va voir si ils font un fix rapidement, sinon, on devra figer la version de sphinx.

(All leaders can lead at least one activity)
Copy link
Member

@jnguiot jnguiot left a comment

Choose a reason for hiding this comment

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

Félicitations pour le boulot, c'est un gros morceau!

@@ -16,6 +16,11 @@
{# Wysiwyg Editor #}
<link rel="stylesheet" href="https://cdn.jsdelivr.net/simplemde/latest/simplemde.min.css">
<script src="https://cdn.jsdelivr.net/simplemde/latest/simplemde.min.js"></script>

{# Multi-select #}
<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/choices.js/public/assets/styles/choices.min.css">
Copy link
Member

Choose a reason for hiding this comment

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

Cette librairie rends super bien, c'est top!

@@ -143,9 +144,11 @@ def __init__(self, event, multi_activities_mode, *args, **kwargs):
super(EventForm, self).__init__(*args, **kwargs)

if "obj" in kwargs:
self.type.data = int(kwargs["obj"].activity_types[0].id)
activities = kwargs["obj"].activity_types
self.multi_activities_mode.data = len(activities) > 1
Copy link
Member

Choose a reason for hiding this comment

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

Pourquoi faire la distinction entre des events avec qu'une seule activités et les events avec plusieurs? Ca ne serait pas plus simple de considérer tous les events comme multiactivité, même s'ils n'ont qu'une seule activité?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

C'est une histoire de qui est autorisé à encadrer quoi. En gros l'immense majorité des sorties seront à activité unique, donc ca vaut le coup de n'autoriser par défaut que des encadrants de l'activité choisie. En mode multi activité au contraire on peu ajouter des encadrants d'une autre activité (ce qui permet ensuite d'ajouter cette nouvelle activité à la sortie).
Pour résumer les vérifications sont plus strictes en mode activité unique, ce qui permet d'avoir un comportement plus intuitif pour le cas le plus courant. Je vais essayer d'ajouter un message d'aide sur la page d'édition.

Copy link
Member

Choose a reason for hiding this comment

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

Ok.

@fberu fberu added the enhancement New feature or request label Apr 21, 2020
@gdaviet
Copy link
Contributor Author

gdaviet commented Apr 21, 2020

Je vais fermer cette revue et en crée une dernière avec tous les changements depuis master, donc #206 + #207 . Si tout vous va ensuite on pourra merger sur master pour déployer sur test.collectives

@gdaviet gdaviet closed this Apr 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants