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

ZEP-13 : Tribunes libres #3606

Closed
wants to merge 14 commits into from
Closed

ZEP-13 : Tribunes libres #3606

wants to merge 14 commits into from

Conversation

gustavi
Copy link
Contributor

@gustavi gustavi commented May 12, 2016

Q R
Type de modification nouvelle fonctionnalité
Ticket(s) (issue(s)) concerné(s) ZEP-13

Présentation

Vous en avez rêvez, elle est là ! Cette PR ajoute le support des Tribunes libres à Zeste de Savoir.

QA

Il faut faire un énorme code review mais également tester le site et tout ce qui a été ajouté. Devant la complexité du code du module zds.tutorialv2 j'ai obligatoirement oublié certaines choses ou mal codé d'autres. Il faut également tester la recherche comme il faut (j'ai fait du copier/coller mais je ne sais pas si ça fonctionne bien).

LA QA PEUT ÊTRE FAITE !

Avant de merger, les derniers détails

  • Ajouter une icône pour les tribunes (si un dev front peut me hl via MP/IRC/ici)

Toutes les remarques/critiques sont les bienvenues, j'ai essayé de faire du code modulable pour qu'on puisse facilement le modifier.

@firm1
Copy link
Contributor

firm1 commented May 12, 2016

\o/

Le jeu. 12 mai 2016 19:34, Coveralls notifications@github.com a écrit :

[image: Coverage Status] https://coveralls.io/builds/6156816

Coverage decreased (-17.1%) to 70.456% when pulling a9d32d4
a9d32d4
on gustavi:zep-13-b
into 195a586
195a586
on zestedesavoir:dev
.


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#3606 (comment)

@@ -112,7 +114,7 @@ <h2 class="home-heading heading-white ico-after ico-news" itemprop="name">
</div>
</section>
<div class="home-row">
<section itemscope itemtype="http://schema.org/ItemList">
<section itemscope itemtype="http://schema.org/ItemList">
Copy link
Member

Choose a reason for hiding this comment

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

Micro mega détail mais bon, un espace en trop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

En faite non il en manquait une ;)

@Emeric54
Copy link
Contributor

Est-ce que pour faciliter les tests, l'un de vous pourrez mettre ça sur son instance ? @gustavi @artragis @sandhose

@@ -8,6 +8,8 @@ Vocabulaire et définitions
- **Contenu** (*content*): désigne, de manière générale, tout ce qui peut être produit et édité sur Zeste de Savoir, c'est-à-dire, à l'heure actuelle, un article ou un tutoriel. Tout contenu est rattaché à un dossier qui lui est propre et dont l'organisation est explicitée plus bas. Ce dossier comporte des informations sur le contenu lui-même (*metadata* : un auteur, une description, une licence...) ainsi que des textes, agencés dans une arborescence bien précise.
- **Article** : contenu, généralement court, visant à faire découvrir un sujet plus qu'à l'expliquer au lecteur (introduit sans rentrer dans les détails) ou à fournir un état des lieux sur un point donné de manière concise (rapports de *release*, actualité...).
- **Tutoriel** : contenu, en général plus long, ayant pour objectif d'enseigner un savoir-faire au lecteur.
- **Tribune libre** : ensemble de billets associé à un utilisateur.
Copy link
Contributor

Choose a reason for hiding this comment

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

Je ne sais pas si il est mieux d'accord avec Tribune ou billets. J'aurait accordé avec billets, donc avec un s à associé.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pour le coup j'avais accordé en fonction de « ensemble » mais on va mettre un « s » histoire de.

@SpaceFox
Copy link
Contributor

Je serais pour remplacer systématiquement « Tribune libre » par « Tribune » dans les messages à destination de l'utilisateur, c'est moins lourd.

"""
Redefinition of `LastContentFeedRSS` for opinions only
"""
content_type = "OPIJION"
Copy link
Contributor

Choose a reason for hiding this comment

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

OPIJION

plutôt OPINION

@gustavi
Copy link
Contributor Author

gustavi commented May 13, 2016

@SpaceFox pas d'objection, ça me convient.

@Emeric54 une version est dispo sur http://gustavi.org:8765/ avec les fixtures de base et c'est tout (pas de mails, pas de recherche, pas de génération des PDF, etc).

@firm1
Copy link
Contributor

firm1 commented May 13, 2016

En dehors de mes remarques de lecture en diagonale, je note aussi le manque dans cette PR de la mise à jour de la page de CGU (il faut pourtant le faire au moins pour préciser la responsabilité du contenu dans les billets)

@gustavi
Copy link
Contributor Author

gustavi commented May 13, 2016

@firm1 https://github.com/zestedesavoir/zds-site/pull/3606/files#diff-d71b710f1375151d343788dafc6a61bf (avec une petite faute que je vais corriger de suite)

@firm1
Copy link
Contributor

firm1 commented May 13, 2016

@firm1 https://github.com/zestedesavoir/zds-site/pull/3606/files#diff-d71b710f1375151d343788dafc6a61bf (avec une petite faute que je vais corriger de suite)

Effectivement, je l'avais pas vu

@gustavi
Copy link
Contributor Author

gustavi commented May 13, 2016

C'est mis à jour. En revanche je n'arrive pas à identifier ce qui casse le TU :/

{% endif %}

{% for author in content.authors.all %}
Copy link
Member

Choose a reason for hiding this comment

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

il faut changer content.authors.all par content.public_version.authors.all et le test unitaire passera !

Copy link
Member

Choose a reason for hiding this comment

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

j'ai fait la PR pour te faire gagner du temps.

@gustavi
Copy link
Contributor Author

gustavi commented May 16, 2016

La QA peut vraiment commencer ici si on veut que ça soit bon pour la v19 ! Je rappelle qu'un dev front doit vérifier mon "travail" et faire quelques ajouts (cc @sandhose @orandin @Situphen )

</li>
{% endif %}
{% if profile.user == user %}
{% if opinions_draft_count > 0 %}
Copy link
Member

Choose a reason for hiding this comment

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

Hum, je me demande si ça ne vaut pas le coup de mettre {% if opinions_draft_count > 0 %} en premier vu qu'il est aussi utilisé dans le else. Ça adoucirait un tout petit peu le code je pense. :)

@LudoBike
Copy link
Contributor

LudoBike commented Nov 1, 2016

Je ne sais pas pourquoi il y a ce bandeau vert vide, je crois que c'est dû à une promotion en article mais ça marche pour les autres billets
selection_007

@LudoBike
Copy link
Contributor

LudoBike commented Nov 1, 2016

selection_007

"Je" ? C'est un membre du staff qui le fait pas Clem

"Un de vos billet a été promu en article" serait plus courtois

@LudoBike
Copy link
Contributor

LudoBike commented Nov 1, 2016

selection_007

Une bêta, où ça ?

Il n'y a pas de bouton de mise en bêta sur le côté comme pour les tuto/articles

@AmarOk1412
Copy link
Member

2016-11-01-15 54 25-screenshot

Publier Publier dans la fenetre de publication

@AmarOk1412
Copy link
Member

Alors je ne sais pas si le contenu est normal, cf https://beta.zestedesavoir.com/tribunes/1596/yolo/
Cette tribune n'apparait pas sur mon profil parce que je me suis enlevé des auteurs. Mais apparaît sur le profil de Situphen.

En gros j'ai créé une tribune, ajouté Situphen comme auteur, publié et je me suis enlevé des auteurs. Je vois donc 2 trucs :

  1. Je ne suis plus auteur, je devrais donc être enlevé des auteurs sur la page ?
  2. Si le probleme 1. est valide, ça veut dire que dans l'état on peut publier une tribune avec l'identité de n'importe qui ?

Enfin dans tous les cas ça me parait un peu bizarre comme comportement

@AmarOk1412
Copy link
Member

Pour rejeter la validation en article, on peut rejeter sans entrer de version et un avertissement nous dit qu'il faut donner une raison après coup. Pourquoi ne pas juste mettre une obligation de mettre une raison ?

@AmarOk1412
Copy link
Member

AmarOk1412 commented Nov 1, 2016

Pour une section :
2016-11-01-17 30 42-screenshot

Et la suppression marche (dans le sens supprime le tuto) mais me retourne une erreur serveur

@sandhose
Copy link
Contributor

sandhose commented Nov 2, 2016

Pour une section :
2016-11-01-17 30 42-screenshot

@AmarOk1412: lié à #3457

@gasche
Copy link
Contributor

gasche commented Nov 12, 2016

Ping gentil? Je suis impatient que cette fonctionnalité devienne disponible car j'aimerais bien écrire des tribunes sur ZDS de temps en temps.

@sandhose
Copy link
Contributor

@gustavi J'ai commencé à regarder pour le front ; j'aimerais bien mieux gérer le menu, du coup je suis en train de voir pour refaire ça en partie et correctement: ça va peut-être me prendre un peu de temps, mais ça sera fait dans la semaine

@vhf
Copy link
Contributor

vhf commented Nov 20, 2016

Cette PR : #3993 , basée sur la branche zds-site/zep-13-b, succède à la présente PR.

Comme ça on peut tous travailler dessus, ça ira plus vite.

J'ai rebasé sur dev puis cherry-pické Corrections RC2.

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.

None yet