From f7795f85ed66fcebaf8a96bf4e550b069fcf1e83 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: [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#61003 Signed-off-by: Thibault Delavallee (tde) --- addons/website_slides/models/slide_channel.py | 20 ++++++++++++++++---- addons/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 7fa0877d8dc..2d70e8943ac 100644 --- a/addons/website_slides/models/slide_channel.py +++ b/addons/website_slides/models/slide_channel.py @@ -42,7 +42,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: record.completed = True partner_karma[record.partner_id.id] += record.channel_id.karma_gen_channel_finish @@ -423,16 +423,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 6510b26cbd0..77d31aa4dab 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") -- cgit v1.2.3