From a3aa45414baaed99a4a986db5979cddde67f13d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Warnon?= Date: Thu, 29 Oct 2020 15:27:47 +0000 Subject: [PATCH] [FIX] website_slides: fix archive/unarchive slide.channel behavior PURPOSE This commit fixes the behavior of archiving and un-archiving slide.channels. SPECS When archiving a slide.channel, you could run into an issue where the associated slide.channel.partners would be marked as "completed" even though the related slides were NOT completed by the user. This happened because when computing the completion, it's searching for slide.slide.partners marked as completed, and then comparing it with the count of slide.slides in the channel, which would always return 0 since all slides are archived when the channel is archived. Marking channels as completed when they should not triggers undesired side-effects (karma increase, completion email sent, ...). Now, when the slide.channel is not active, we don't mark associated slide.channel.partners as completed anymore. The ordering in which we archive channel and slides was also altered, see method docstring of 'slide.channel#toggle_active' for more info. A new test ensures the correct behavior of archiving. LINKS Task 2170708 closes odoo/odoo#61773 X-original-commit: f7795f85ed66fcebaf8a96bf4e550b069fcf1e83 Signed-off-by: Thibault Delavallee (tde) Signed-off-by: awa-odoo --- addons/website_slides/models/slide_channel.py | 20 +++++++++++++++---- .../website_slides/tests/test_slide_utils.py | 11 ++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/addons/website_slides/models/slide_channel.py b/addons/website_slides/models/slide_channel.py index c733e176118..34579598ed3 100644 --- a/addons/website_slides/models/slide_channel.py +++ b/addons/website_slides/models/slide_channel.py @@ -45,7 +45,7 @@ class ChannelUsersRelation(models.Model): for record in self: record.completed_slides_count = mapped_data.get(record.channel_id.id, dict()).get(record.partner_id.id, 0) record.completion = 100.0 if record.completed else round(100.0 * record.completed_slides_count / (record.channel_id.total_slides or 1)) - if not record.completed and record.completed_slides_count >= record.channel_id.total_slides: + if not record.completed and record.channel_id.active and record.completed_slides_count >= record.channel_id.total_slides: completed_records += record if completed_records: @@ -476,16 +476,28 @@ class Channel(models.Model): return res def toggle_active(self): - # archiving/unarchiving a channel does it on its slides, too + """ Archiving/unarchiving a channel does it on its slides, too. + 1. When archiving + We want to be archiving the channel FIRST. + So that when slides are archived and the recompute is triggered, + it does not try to mark the channel as "completed". + That happens because it counts slide_done / slide_total, but slide_total + will be 0 since all the slides for the course have been archived as well. + + 2. When un-archiving + We want to archive the channel LAST. + So that when it recomputes stats for the channel and completion, it correctly + counts the slides_total by counting slides that are already un-archived. """ + to_archive = self.filtered(lambda channel: channel.active) to_activate = self.filtered(lambda channel: not channel.active) - res = super(Channel, self).toggle_active() if to_archive: + super(Channel, to_archive).toggle_active() to_archive.is_published = False to_archive.mapped('slide_ids').action_archive() if to_activate: to_activate.with_context(active_test=False).mapped('slide_ids').action_unarchive() - return res + super(Channel, to_activate).toggle_active() @api.returns('mail.message', lambda value: value.id) def message_post(self, *, parent_id=False, subtype_id=False, **kwargs): diff --git a/addons/website_slides/tests/test_slide_utils.py b/addons/website_slides/tests/test_slide_utils.py index 9fa7893fd71..830ef0dfc82 100644 --- a/addons/website_slides/tests/test_slide_utils.py +++ b/addons/website_slides/tests/test_slide_utils.py @@ -25,8 +25,17 @@ class TestSlidesManagement(slides_common.SlidesCase): @users('user_manager') def test_archive(self): + self.env['slide.slide.partner'].create({ + 'slide_id': self.slide.id, + 'channel_id': self.channel.id, + 'partner_id': self.user_manager.partner_id.id, + 'completed': True + }) + channel_partner = self.channel._action_add_members(self.user_manager.partner_id) + self.assertTrue(self.channel.active) self.assertTrue(self.channel.is_published) + self.assertFalse(channel_partner.completed) for slide in self.channel.slide_ids: self.assertTrue(slide.active, "All slide should be archived when a channel is archived") self.assertTrue(slide.is_published, "All slide should be unpublished when a channel is archived") @@ -34,6 +43,8 @@ class TestSlidesManagement(slides_common.SlidesCase): self.channel.toggle_active() self.assertFalse(self.channel.active) self.assertFalse(self.channel.is_published) + # channel_partner should still NOT be marked as completed + self.assertFalse(channel_partner.completed) for slide in self.channel.slide_ids: self.assertFalse(slide.active, "All slides should be archived when a channel is archived")