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

feat(forum): harmoniser la liste des topics #665

Merged
merged 10 commits into from
Jun 12, 2024

Conversation

calummackervoy
Copy link
Contributor

@calummackervoy calummackervoy commented Jun 10, 2024

Description

🎸 soutenir filtration sur ForumView
🎸 supprimer ForumTopicListView, l'accès pour HTMX fournis par ForumView
🎸 FilteredTopicsListViewMixin pour éviter la répétition de logique entre les views du Forum et l'espace d'échanges

Type de changement

🎢 Nouvelle fonctionnalité (changement non cassant qui ajoute une fonctionnalité).
🥁 Changement de rupture (modification ou caractéristique qui empêcherait une fonctionnalité existante de fonctionner comme prévu) nécéssitant une mise à jour de la documentation

Captures d'écran (optionnel)

Screenshot 2024-06-10 at 17 23 21 Screenshot 2024-06-10 at 17 23 40 Screenshot 2024-06-10 at 17 24 05

@calummackervoy calummackervoy linked an issue Jun 10, 2024 that may be closed by this pull request
@vincentporte
Copy link
Contributor

Merci @calummackervoy , super interessant le mixin.
Est ce que tu pourrais harmoniser l'UI entre /topics/ et /forum/<slug>-<id>/ ?

Capture d’écran du 2024-06-11 09-17-54
Capture d’écran du 2024-06-11 09-17-34

@vincentporte
Copy link
Contributor

Je viens de trouver ce bug en cours de revue : #667
Il existe déjà dans la version déployée

Copy link
Contributor

@vincentporte vincentporte left a comment

Choose a reason for hiding this comment

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

Super refactor 🤝

lacommunaute/forum/views.py Show resolved Hide resolved
lacommunaute/forum/views.py Outdated Show resolved Hide resolved
lacommunaute/forum/tests/tests_views.py Show resolved Hide resolved
@vincentporte vincentporte linked an issue Jun 11, 2024 that may be closed by this pull request
@calummackervoy calummackervoy force-pushed the 618-harmoniser-la-liste-des-topics branch 2 times, most recently from 3f68a0e to d0eb950 Compare June 11, 2024 13:16
Copy link
Contributor

@francoisfreitag francoisfreitag left a comment

Choose a reason for hiding this comment

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

Quelques idées pour simplifier, éviter d’ajouter des requêtes SQL (voir en enlever).

lacommunaute/forum_conversation/view_mixins.py Outdated Show resolved Hide resolved
lacommunaute/forum_conversation/view_mixins.py Outdated Show resolved Hide resolved
Comment on lines 19 to 20
if self.get_filter() == Filters.NEW:
qs = qs.unanswered()
elif self.get_filter() == Filters.CERTIFIED:
qs = qs.filter(certified_post__isnull=False)

if self.get_tags():
qs = qs.filter(tags__in=self.get_tags())
Copy link
Contributor

Choose a reason for hiding this comment

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

ℹ️ Une autre abstraction possible serait d’utiliser un Form pour filtrer le queryset en fonction de request.GET, ce qui permettrait de valider les champs au lieu d’utiliser des lookups qui supportent des données invalides.

Parmis les bugs possibles dans une future version, ajouter une méthode à Filters (dans l’idée de BlockedPostReason.reasons_tracked_for_stats()) l’exposerait aux requêtes des utilisateurs.

lacommunaute/forum/tests/tests_views.py Show resolved Hide resolved
lacommunaute/forum_conversation/view_mixins.py Outdated Show resolved Hide resolved
lacommunaute/forum_conversation/view_mixins.py Outdated Show resolved Hide resolved
lacommunaute/forum_conversation/view_mixins.py Outdated Show resolved Hide resolved
lacommunaute/forum_conversation/views.py Outdated Show resolved Hide resolved
lacommunaute/forum_conversation/view_mixins.py Outdated Show resolved Hide resolved
lacommunaute/forum_conversation/views.py Outdated Show resolved Hide resolved
@vincentporte vincentporte self-requested a review June 12, 2024 08:45
Copy link
Contributor

@vincentporte vincentporte left a comment

Choose a reason for hiding this comment

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

@calummackervoy , nous avons déjà passé pas mal de temps sur cette PR. Je te laisse voir pour les dernières remarques de François sur get_url_encoded_params et get_filter. Tu peux déployer ensuite

@calummackervoy
Copy link
Contributor Author

OK :) merci pour vos supers revues !

@calummackervoy calummackervoy force-pushed the 618-harmoniser-la-liste-des-topics branch from 1ff760d to b4d3c86 Compare June 12, 2024 13:28
@calummackervoy calummackervoy merged commit 6f8bf3c into master Jun 12, 2024
8 checks passed
@calummackervoy calummackervoy deleted the 618-harmoniser-la-liste-des-topics branch June 12, 2024 13:33
vincentporte pushed a commit that referenced this pull request Jun 13, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.10.0](v2.9.2...v2.10.0)
(2024-06-13)


### Features

* **admin:** ajoute PostInline à TopicAdmin
([#657](#657))
([6f3049a](6f3049a))
* ajout du chemin de fer dans l'espace d'échanges et la documentation
([#649](#649))
([7cc5d14](7cc5d14))
* **forum_conversation/admin:** CertifiedPost verbose_name et traduction
([#666](#666))
([e9b0da6](e9b0da6))
* **forum_conversation/tests.py:** test explicite pour la seconde page
([#669](#669))
([5bea4f1](5bea4f1))
* **forum_conversation:** remettre les annonces dans la liste des
messages par défaut
([#662](#662))
([bb341cb](bb341cb))
* **forum_moderation:** enregistrement des messages bloquées
([#659](#659))
([7318b5d](7318b5d))
* **forum:** harmoniser la liste des topics
([#665](#665))
([6f8bf3c](6f8bf3c))
* **stats:** afficher les stats quotidiennes des Diagnostics Parcours
IAE
([#660](#660))
([5c74c56](5c74c56))
* **stats:** ajout de la vue historique des visiteurs mensuels
([#656](#656))
([804be3c](804be3c))
* **stats:** collecter le nombre de diag parcours iae réalisés
quotidiennement
([#658](#658))
([7205a2b](7205a2b))


### Bug Fixes

* **documentation:** améliorer l'affichage des bannières des fiches
pratiques
([#653](#653))
([3b539d2](3b539d2))
* **forum_conversation:** page 1 forcée dans les liens tags
([#668](#668))
([a150ab2](a150ab2))
* **forum_moderation:** motif de blocage illisible dans l'admin
([#672](#672))
([5480394](5480394))
* **quality:** formatage des balises `script` dans le gabarit
`base.html`
([#651](#651))
([0be3c8b](0be3c8b))
* **stats:** corrections mineures
([#661](#661))
([d3ae4fd](d3ae4fd))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

[FORUM] harmoniser la liste des topics [TOPICS] harmoniser la vue /topics/ et les vues /forum/xx-id/
3 participants