From 9f39ca12ae57f341b67bd73f8dc1e4a3f8110ed5 Mon Sep 17 00:00:00 2001 From: Fabio Barbero Date: Tue, 14 Jun 2022 13:41:19 +0000 Subject: [PATCH] [FIX] knowledge: fix resequencing articles Purpose ======= Properly display and resequence articles based on sequence field. Articles are currently not sorted based on sequence, and when moving articles outside of a parent resequencing is not applied. Specifications ============== Properly resequence articles not only if the parent_id value is non-false but also if it's in the vals and set to False (when moving an article to the root). Task-2883368 closes odoo/odoo#95078 X-original-commit: 6dde05e8779feb2ad6f8828e1bc2945fcf403248 Signed-off-by: Thibault Delavallee (tde) --- addons/knowledge/controllers/main.py | 3 +-- addons/knowledge/models/knowledge_article.py | 26 ++++++++++++------- .../tests/test_knowledge_article_sequence.py | 19 ++++++++++++++ 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/addons/knowledge/controllers/main.py b/addons/knowledge/controllers/main.py index 8c4fb016558..25bb4b02f78 100644 --- a/addons/knowledge/controllers/main.py +++ b/addons/knowledge/controllers/main.py @@ -95,8 +95,7 @@ class KnowledgeController(http.Controller): def _get_root_articles(self, limit=None): """ Meant to be overriden by website_knowledge to search in sudo with adapted domain.""" - _order = 'sequence' if limit is not None else None - return request.env["knowledge.article"].search([("parent_id", "=", False)], limit=limit, order=_order) + return request.env["knowledge.article"].search([("parent_id", "=", False)], limit=limit, order='sequence, id') def _prepare_articles_tree_html(self, template, active_article, unfolded_articles=False): """ Prepares all the info needed to render the article tree view side panel diff --git a/addons/knowledge/models/knowledge_article.py b/addons/knowledge/models/knowledge_article.py index fcb0e6c22a4..e8d2c70a708 100644 --- a/addons/knowledge/models/knowledge_article.py +++ b/addons/knowledge/models/knowledge_article.py @@ -608,14 +608,16 @@ class Article(models.Model): def write(self, vals): # Move under a parent is considered as a write on it (permissions, ...) _resequence = False - if vals.get('parent_id'): - parent = self.browse(vals['parent_id']) - try: - parent.check_access_rights('write') - parent.check_access_rule('write') - except AccessError: - raise AccessError(_("You cannot move an article under %(parent_name)s as you cannot write on it", - parent_name=parent.display_name)) + if 'parent_id' in vals: + parent = self.env['knowledge.article'] + if vals.get('parent_id'): + parent = self.browse(vals['parent_id']) + try: + parent.check_access_rights('write') + parent.check_access_rule('write') + except AccessError: + raise AccessError(_("You cannot move an article under %(parent_name)s as you cannot write on it", + parent_name=parent.display_name)) if 'sequence' not in vals: max_sequence = self._get_max_sequence_inside_parents(parent.ids).get(parent.id, -1) vals['sequence'] = max_sequence + 1 @@ -744,7 +746,7 @@ class Article(models.Model): :param bool is_private: set as private; """ self.ensure_one() - parent = self.browse(parent_id) + parent = self.browse(parent_id) if parent_id else self.env['knowledge.article'] before_article = self.browse(before_article_id) values = {'parent_id': parent_id} @@ -838,8 +840,12 @@ class Article(models.Model): @api.model def _get_max_sequence_inside_parents(self, parent_ids): max_sequence_by_parent = {} + if parent_ids: + domain = [('parent_id', 'in', parent_ids)] + else: + domain = [('parent_id', '=', False)] rg_results = self.env['knowledge.article'].sudo().read_group( - [('parent_id', 'in', parent_ids)], + domain, ['sequence:max'], ['parent_id'] ) diff --git a/addons/knowledge/tests/test_knowledge_article_sequence.py b/addons/knowledge/tests/test_knowledge_article_sequence.py index 4877e38144d..eda6e684195 100644 --- a/addons/knowledge/tests/test_knowledge_article_sequence.py +++ b/addons/knowledge/tests/test_knowledge_article_sequence.py @@ -144,6 +144,25 @@ class TestKnowledgeArticleSequence(KnowledgeCommon): self.assertSortedSequence(article_private + article_private2) self.assertSortedSequence(article_children[2] + article_children[3] + article_children[1] + article_children[0]) + @users('employee') + def test_resequence_with_move_noparent(self): + """ Test move resetting parent_id should also compute sequence """ + article_private = self.article_private.with_env(self.env) + article_private_child = self.article_children[0].with_env(self.env) + article_private2 = self.article_private2.with_env(self.env) + article_root_noise = self.article_root_noise.with_env(self.env) + + self.assertEqual(article_private_child.sequence, 0) + article_private_child.move_to(parent_id=False) + self.assertEqual(article_private_child.sequence, 6) + self.assertSortedSequence(article_root_noise + article_private + article_private2 + article_private_child) + article_private_child.move_to( + parent_id=False, + before_article_id=self.article_root_noise[0].id + ) + self.assertEqual(article_private_child.sequence, 1) + self.assertSortedSequence(article_private_child + article_root_noise + article_private + article_private2) + @users('employee') def test_resequence_with_parent(self): """Checking the sequence of the articles"""