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

Corrige la route pour dissocier un ticket GitHub d'un sujet du forum #6378

Merged
merged 2 commits into from
Aug 31, 2022

Conversation

philippemilink
Copy link
Member

Cette PR corrige la route pour dissocier un ticket GitHub d'un sujet du forum (probablement un oubli lors de la refactorisation des URLs) et ajoute un test pour s'assurer que ce genre de problème ne passe plus sous nos radars.

Problème rapporté par Sentry.

QA

Le plus simple est sans doute de faire une revue de code et de constater que le changement principal est correct et ne peut pas introduire de bug, sinon il est possible que je déploie cette PR sur la bêta. Si #6377 est fusionné lors de la QA, il est aussi possible modifier les champs Ticket GitHub (avec n'importe quelle valeur) et Nom du dépôt GitHub (par une des valeurs des dépôts définies dans la configuration) et en tant qu'utilisateur dev, aller sur la page du sujet et constater qu'elle s'affiche correctement.

@coveralls
Copy link

coveralls commented Aug 13, 2022

Coverage Status

Coverage increased (+0.03%) to 88.206% when pulling 69aa791 on philippemilink:fix-url into bf30207 on zestedesavoir:dev.

Copy link
Contributor

@Arnaud-D Arnaud-D left a comment

Choose a reason for hiding this comment

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

Je pense que ce n'est pas la peine de QA outre mesure, c'est juste un oubli que j'ai fais lors de la refactorisation des URL. Ce genre de soucis est attrapé par le type de test que tu as ajouté et dont on manque globalement dans la suite.

Comment on lines 1930 to 1977
class ManageGitHubIssueTest(TestCase):
def test_show_links(self):
"""
Test if buttons to manage GitHub issues are correctly displayed according to the context.
"""
profile = ProfileFactory()
dev_profile = DevProfileFactory()
category, forum = create_category_and_forum()
unlinked_topic = create_topic_in_forum(forum, profile)
linked_topic = create_topic_in_forum(forum, profile)
linked_topic.github_repository_name = settings.ZDS_APP["github_projects"]["repositories"][0]
linked_topic.github_issue = 42
linked_topic.save()

route = "forum:topic-posts-list"
label_linked_issue = _("Ticket associé")
label_unlink_issue = _("Dissocier le ticket")
label_create_issue = _("Créer un ticket")
label_link_issue = _("Associer un ticket")

# Default: nothing is displayed
response = self.client.get(reverse(route, args=[unlinked_topic.pk, unlinked_topic.slug()]))
self.assertNotContains(response, label_linked_issue)
self.assertNotContains(response, label_unlink_issue)
self.assertNotContains(response, label_create_issue)
self.assertNotContains(response, label_link_issue)

# An issue is associated: the link is displayed
response = self.client.get(reverse(route, args=[linked_topic.pk, linked_topic.slug()]))
self.assertContains(response, label_linked_issue)
self.assertNotContains(response, label_unlink_issue)
self.assertNotContains(response, label_create_issue)
self.assertNotContains(response, label_link_issue)

# Logged as a dev member, no ticket is associated: buttons to link or create an issue are displayed
self.client.force_login(dev_profile.user)
response = self.client.get(reverse(route, args=[unlinked_topic.pk, unlinked_topic.slug()]))
self.assertNotContains(response, label_linked_issue)
self.assertNotContains(response, label_unlink_issue)
self.assertContains(response, label_create_issue)
self.assertContains(response, label_link_issue)

# Logged as a dev member, a ticket is associated: buttons to unlink or view the issue are displayed
response = self.client.get(reverse(route, args=[linked_topic.pk, linked_topic.slug()]))
self.assertContains(response, label_linked_issue)
self.assertContains(response, label_unlink_issue)
self.assertNotContains(response, label_create_issue)
self.assertNotContains(response, label_link_issue)
Copy link
Contributor

Choose a reason for hiding this comment

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

Je pense que c'est mieux de séparer les différents cas. En fait, tes commentaires délimitent les cas qui devraient être dans des tests individuels. Et ce qui est au début de la fonction est le setUp. ^^

Copy link
Member Author

Choose a reason for hiding this comment

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

Corrigé.

@Arnaud-D Arnaud-D enabled auto-merge (squash) August 31, 2022 13:04
@Arnaud-D Arnaud-D disabled auto-merge August 31, 2022 13:04
@Arnaud-D Arnaud-D enabled auto-merge (squash) August 31, 2022 13:06
@Arnaud-D Arnaud-D merged commit 15795fc into zestedesavoir:dev Aug 31, 2022
@philippemilink philippemilink deleted the fix-url branch April 10, 2023 17:03
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.

3 participants