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

Rend générique en fonction du type la vue "WarnTypo" #6669

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

Arnaud-D
Copy link
Contributor

Dans le cadre de la nouvelle organisation des contenus

  • supprime la distinction de type de publication
  • préfère le terme "publication" à "contenu"
  • renomme les vues pour adopter la convention de Django
  • déplace le formulaire à côté de la vue

Contrôle qualité

Tester le formulaire de signalement de faute sur les différents types de contenu, regarder les MP correspondants.

@Arnaud-D Arnaud-D added the C-Back Concerne le back-end Django label Oct 19, 2024
@coveralls
Copy link

coveralls commented Oct 19, 2024

Coverage Status

coverage: 89.214% (+0.009%) from 89.205%
when pulling 3983935 on Arnaud-D:retire-des-distinctions-entre-types-3
into fe84653 on zestedesavoir:dev.

@Arnaud-D Arnaud-D marked this pull request as ready for review October 19, 2024 08:17
@Arnaud-D Arnaud-D force-pushed the retire-des-distinctions-entre-types-3 branch from 5b05c04 to d6e9bb7 Compare November 11, 2024 10:52
@Arnaud-D Arnaud-D force-pushed the retire-des-distinctions-entre-types-3 branch from d6e9bb7 to 7526887 Compare December 25, 2024 08:50
@firm1
Copy link
Contributor

firm1 commented Jan 22, 2025

Rapport de QA

QA OK 👍

Je n'ai pas pris le temps de faire la review, donc à review avant merge.


pm_title = _("J'ai trouvé une faute dans {} « {} ».").format(_type, form.content.title)

pm_title = _("J'ai trouvé une faute dans « {} ».").format(form.content.title)
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Les deux ne sont pas tout à fait pareil et je n'ai pas le courage de d'harmoniser sans risquer de casser quelque chose.

messages.error(self.request, _("Impossible d'envoyer la proposition de correction : vous êtes auteur."))

else: # send correction
else: # Send correction
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
else: # Send correction
else: # Send PM

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 refactorisé différemment et ça rend ce commentaire superflu.

self._errors["text"] = self.error_class([_("Vous devez indiquer la faute commise.")])
if "text" in cleaned_data:
del cleaned_data["text"]

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fait.

Comment on lines 119 to 121
version = content.sha_beta
if public:
version = content.sha_public
Copy link
Member

Choose a reason for hiding this comment

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

On peut sans doute factoriser cette portion du code avec la définition plus haut de self.previous_page_url (il y a le même if public).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fait.

Comment on lines 109 to 110
num_of_authors = content.authors.count()
for index, user in enumerate(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 faudrait récupérer uniquement les membres contactables (cf https://github.com/zestedesavoir/zds-site/pull/6669/files#diff-4b06fa578d536b8bff461bfdd2196ac3afa8179198ec0fe96319d859d9b947d3R185). Ça me fait donc penser qu'on pourrait ajouter une méthode qui récupère les auteurs contactables dans la classe PublishableContent ?

Dans tous les cas, je pense qu'il est possible de factoriser ce code avec ce qu'il y a dans la vue.

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 préféré prendre la notion de "reachable" présente dans les MP. Il y a aussi des histoires de "contactable" ailleurs dans le code, mais c'est un peu le chaos. Harmoniser les deux notions est un ticket à part entière (#5696).

JsFiddleActivationForm,
PublicationForm,
PickOpinionForm,
UnpickOpinionForm,
PromoteOpinionToArticleForm,
)
from zds.tutorialv2.views.misc import WarnTypoForm
Copy link
Member

Choose a reason for hiding this comment

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

En regardant dans ce fichier pourquoi on avait besoin de cet import, je me rends compte que cette ligne :

context["form_warn_typo"] = WarnTypoForm(versioned, versioned, public=False)
est dupliquée dix lignes plus loin sans raison apparente :
context["form_warn_typo"] = WarnTypoForm(self.versioned_object, self.versioned_object)

On peut sans doute profiter de cette PR pour corriger ça.

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 corrigé ça.

* supprime la distinction de type de publication
* préfère le terme "publication" à "contenu"
* renomme les vues pour adopter la convention de Django
* déplace le formulaire à côté de la vue
@Arnaud-D Arnaud-D force-pushed the retire-des-distinctions-entre-types-3 branch from 8ca6ccb to 3983935 Compare February 2, 2025 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Back Concerne le back-end Django
Projects
Status: En attente de QA
Development

Successfully merging this pull request may close these issues.

4 participants