[IMP] academic: next year groups dedup and user feedback

academic/__manifest__.py

@@ -19,7 +19,7 @@
 ##############################################################################
 {
     "name": "Academic",
-    "version": "19.0.1.13.0",
+    "version": "19.0.1.14.0",
     "sequence": 14,
     "summary": "",
     "author": "ADHOC SA",

academic/migrations/19.0.1.14.0/post-migration.py

@@ -0,0 +1,21 @@
+def migrate(cr, version):
+    # the old m2m had no order: the sequence follows the level ids and the admin
+    # reviews it by dragging the lines on the study plan
+    cr.execute("SELECT 1 FROM information_schema.tables WHERE table_name = 'academic_section_level_ids_rel'")
+    if not cr.fetchone():
+        return
+    cr.execute(
+        """
+        INSERT INTO academic_section_level (section_id, level_id, sequence, create_uid, create_date, write_uid, write_date)
+             SELECT rel.section_id,
+                    rel.level_id,
+                    10 * row_number() OVER (PARTITION BY rel.section_id ORDER BY rel.level_id),
+                    1, now(), 1, now()
+               FROM academic_section_level_ids_rel rel
+              WHERE NOT EXISTS (
+                        SELECT 1
+                          FROM academic_section_level line
+                         WHERE line.section_id = rel.section_id
+                           AND line.level_id = rel.level_id)
+        """
+    )

academic/models/__init__.py

@@ -7,6 +7,7 @@
 from . import academic_level
 from . import academic_promotion
 from . import academic_section
+from . import academic_section_level
 from . import academic_subject_template
 from . import academic_subject
 from . import hr

academic/models/academic_group.py

@@ -93,29 +93,77 @@ def _compute_name(self):
             ]
             line.name = " - ".join(filter(None, name_parts))
 
+    def _get_next_year_group(self, level=None):
+        """`level` overrides the level to search for, to follow the study plan sequence."""
+        self.ensure_one()
+        # active_test=False: the unique constraint ignores `active`, so an archived group
+        # that is not found here makes the copy below crash on a unique violation
+        return (
+            self.env["academic.group"]
+            .with_context(active_test=False)
+            .search(
+                [
+                    ("year", "=", self.year + 1),
+                    ("company_id", "=", self.company_id.id),
+                    ("section_id", "=", self.section_id.id),
+                    ("level_id", "=", (level or self.level_id).id),
+                    ("division_id", "=", self.division_id.id),
+                    ("subject_id", "=", self.subject_id.id),
+                ],
+                limit=1,
+            )
+        )
+
+    def _create_next_year_group(self, level=None):
+        self.ensure_one()
+        return self.copy(
+            default={
+                "year": self.year + 1,
+                "level_id": (level or self.level_id).id,
+                "student_ids": False,
+            }
+        )
+
+    def _get_or_create_next_year_group(self, level=None):
+        self.ensure_one()
+        return self._get_next_year_group(level=level) or self._create_next_year_group(level=level)
+
+    def _get_groups_action(self):
+        action = self.env["ir.actions.actions"]._for_xml_id("academic.action_academic_group_groups")
+        action.update({"domain": [("id", "in", self.ids)], "context": {}})
+        return action
+
+    def _get_next_year_level(self):
+        """Next level in the study plan. Empty when the group closes the plan (its students
+        are not re-enrolled), the same level when the plan has no sequence configured."""
+        self.ensure_one()
+        if self.section_id._is_last_level(self.level_id):
+            return self.env["academic.level"]
+        return self.section_id._get_next_level(self.level_id) or self.level_id
+
     def create_next_year_groups(self):
         # estamos pasando de un año a otro sin usar study plan por lo siguiente:
         # a) hay muchos colegios que no lo tienen bien implmentado
         # b) los study plan no pueden reflejar todos los casos todavia (por )
-
+        existing = next_groups = self.env["academic.group"]
         for rec in self:
-            next_group = rec.env["academic.group"].search(
-                [
-                    ("year", "=", rec.year + 1),
-                    ("company_id", "=", rec.company_id.id),
-                    ("level_id", "=", rec.level_id.id),
-                    ("division_id", "=", rec.division_id.id),
-                ],
-                limit=1,
-            )
+            found = rec._get_next_year_group()
+            existing |= found
+            next_groups |= found or rec._create_next_year_group()
 
-            if not next_group:
-                next_group = rec.copy(
-                    default={
-                        "year": rec.year + 1,
-                        "student_ids": False,
-                    }
-                )
+        return {
+            "type": "ir.actions.client",
+            "tag": "display_notification",
+            "params": {
+                "type": "success",
+                "message": self.env._(
+                    "%(created)s next year group(s) created, %(existing)s already existed.",
+                    created=len(next_groups - existing),
+                    existing=len(existing),
+                ),
+                "next": next_groups._get_groups_action(),
+            },
+        }
 
     def open_students(self):
         action = self.env.ref("academic.action_academic_partner_students").read()[0]

academic/models/academic_section.py

@@ -2,7 +2,7 @@
 # For copyright and license notices, see __manifest__.py file in module root
 # directory
 ##############################################################################
-from odoo import fields, models
+from odoo import Command, api, fields, models
 
 
 class AcademicSection(models.Model):
@@ -20,13 +20,50 @@ class AcademicSection(models.Model):
         "correlative_id",
         string="Correlative Study Plans",
     )
+    level_line_ids = fields.One2many(
+        "academic.section.level",
+        "section_id",
+        string="Levels Sequence",
+        help="Levels of the study plan, in the order the student goes through them.",
+    )
     level_ids = fields.Many2many(
         "academic.level",
-        "academic_section_level_ids_rel",
-        "section_id",
-        "level_id",
         string="Levels",
+        compute="_compute_level_ids",
+        inverse="_inverse_level_ids",
     )
     sequence = fields.Integer(
         default=10,
     )
+
+    @api.depends("level_line_ids.level_id")
+    def _compute_level_ids(self):
+        for rec in self:
+            rec.level_ids = rec.level_line_ids.level_id
+
+    def _inverse_level_ids(self):
+        """Keeps level_ids usable as a plain m2m (data files, imports, existing views)."""
+        for rec in self:
+            lines = rec.level_line_ids
+            commands = [Command.unlink(line.id) for line in lines if line.level_id not in rec.level_ids]
+            sequence = max(lines.mapped("sequence"), default=0)
+            for level in rec.level_ids - lines.level_id:
+                sequence += 10
+                commands.append(Command.create({"level_id": level.id, "sequence": sequence}))
+            rec.level_line_ids = commands
+
+    def _is_last_level(self, level):
+        """True only when the plan has its sequence configured and `level` closes it, as
+        opposed to a plan with no sequence at all, where nothing can be told apart."""
+        self.ensure_one()
+        lines = self.level_line_ids.sorted(lambda x: (x.sequence, x.id))
+        return bool(lines) and lines[-1].level_id == level
+
+    def _get_next_level(self, level):
+        self.ensure_one()
+        # sorted explicitly: the o2m order can be stale in cache right after writing the sequence
+        lines = self.level_line_ids.sorted(lambda x: (x.sequence, x.id))
+        for line, next_line in zip(lines, lines[1:]):
+            if line.level_id == level:
+                return next_line.level_id
+        return self.env["academic.level"]

academic/models/academic_section_level.py

@@ -0,0 +1,27 @@
+##############################################################################
+# For copyright and license notices, see __manifest__.py file in module root
+# directory
+##############################################################################
+from odoo import fields, models
+
+
+class AcademicSectionLevel(models.Model):
+    _name = "academic.section.level"
+    _description = "Study Plan Level"
+    _order = "sequence, id"
+    _rec_name = "level_id"
+
+    _level_unique = models.Constraint(
+        "unique(section_id, level_id)",
+        "Each level can only be added once per study plan.",
+    )
+
+    section_id = fields.Many2one(
+        "academic.section",
+        string="Study Plan",
+        required=True,
+        ondelete="cascade",
+        index=True,
+    )
+    level_id = fields.Many2one("academic.level", string="Level", required=True)
+    sequence = fields.Integer(default=10)

academic/models/res_partner.py

@@ -321,6 +321,10 @@ def _compute_payment_responsible(self):
             rec.payment_responsible_ids = [(6, 0, partners.ids)]
         (self - students).payment_responsible_ids = False
 
+    def _filter_without_payment_responsible(self):
+        """Students that cannot be billed: no payment responsible, or all of them archived."""
+        return self.filtered(lambda x: not x.payment_responsible_ids.filtered("active"))
+
     @api.constrains("student_link_ids", "self_payment_responsible", "vat")
     def _check_vat_partner_paying_role(self):
         paying_role = self.env.ref("academic.paying_role")

academic/security/ir.model.access.csv

@@ -11,6 +11,9 @@ access_academic_subject_template_manager,academic.subject.template.manager,model
 access_academic_subject_template_user,academic.subject.template.user,model_academic_subject_template,group_user,1,0,0,0
 access_academic_section_manager,academic.section.manager,model_academic_section,group_manager,1,1,1,1
 access_academic_section_user,academic.section.user,model_academic_section,group_user,1,0,0,0
+access_academic_section_level_manager,academic.section.level.manager,model_academic_section_level,group_manager,1,1,1,1
+access_academic_section_level_user,academic.section.level.user,model_academic_section_level,group_user,1,0,0,0
+access_academic_section_level_global,academic.section.level.global,model_academic_section_level,base.group_user,1,0,0,0
 access_academic_promotion_manager,academic.promotion.manager,model_academic_promotion,group_manager,1,1,1,1
 access_academic_promotion_user,academic.promotion.user,model_academic_promotion,group_user,1,1,1,1
 access_academic_division_manager,academic.division.manager,model_academic_division,group_manager,1,1,1,1

academic/tests/__init__.py

@@ -4,3 +4,4 @@
 ##############################################################################
 from . import test_archive_family
 from . import test_contact_import
+from . import test_next_year_groups

academic/tests/test_next_year_groups.py

@@ -0,0 +1,92 @@
+##############################################################################
+# For copyright and license notices, see __manifest__.py file in module root
+# directory
+##############################################################################
+from odoo import Command
+from odoo.tests.common import TransactionCase, tagged
+
+
+@tagged("post_install", "-at_install")
+class TestNextYearGroups(TransactionCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.level_1 = cls.env["academic.level"].create({"name": "Test Next Year Level 1"})
+        cls.level_2 = cls.env["academic.level"].create({"name": "Test Next Year Level 2"})
+        cls.division = cls.env["academic.division"].create({"name": "Test Next Year Division"})
+        cls.section = cls.env["academic.section"].create(
+            {
+                "name": "Test Next Year Plan",
+                "level_line_ids": [
+                    Command.create({"level_id": cls.level_1.id, "sequence": 10}),
+                    Command.create({"level_id": cls.level_2.id, "sequence": 20}),
+                ],
+            }
+        )
+        cls.company = cls.env.company
+        cls.company.section_ids = [Command.link(cls.section.id)]
+        cls.group = cls.env["academic.group"].create(
+            {
+                "year": 2026,
+                "company_id": cls.company.id,
+                "section_id": cls.section.id,
+                "level_id": cls.level_1.id,
+                "division_id": cls.division.id,
+                # academic_sale_subscription constrains capacity > 0, and the copy of the
+                # next year group carries it along, so it cannot be left at the default
+                "capacity": 10,
+            }
+        )
+
+    def _next_year_groups(self, level=None, active_test=True):
+        domain = [("year", "=", 2027), ("section_id", "=", self.section.id)]
+        if level:
+            domain.append(("level_id", "=", level.id))
+        return self.env["academic.group"].with_context(active_test=active_test).search(domain)
+
+    def test_01_running_the_mass_action_twice_creates_one_group(self):
+        """The dedup search is the whole point of the action: re-running it must reuse."""
+        self.group.create_next_year_groups()
+        self.assertEqual(len(self._next_year_groups()), 1)
+
+        self.group.create_next_year_groups()
+        self.assertEqual(len(self._next_year_groups()), 1, "the second run duplicated the next year group")
+
+    def test_02_subject_group_does_not_reuse_the_commercial_group(self):
+        """A subject group and the commercial group of the same level are different
+        records for the unique constraint, so they need one next year group each."""
+        template = self.env["academic.subject.template"].create({"name": "Test Next Year Subject", "code": "TSTNY"})
+        subject = self.env["academic.subject"].create(
+            {"name": "Test Next Year Subject", "company_id": self.company.id, "template_id": template.id}
+        )
+        subject_group = self.group.copy({"subject_id": subject.id})
+
+        (self.group + subject_group).create_next_year_groups()
+
+        next_groups = self._next_year_groups()
+        self.assertEqual(len(next_groups), 2)
+        self.assertEqual(len(next_groups.filtered("subject_id")), 1)
+        self.assertEqual(len(next_groups.filtered(lambda x: not x.subject_id)), 1)
+
+    def test_03_archived_next_year_group_is_reused(self):
+        """The unique constraint ignores `active`: not finding an archived group would
+        make the copy blow up on a unique violation and abort the whole batch."""
+        self.group.create_next_year_groups()
+        self._next_year_groups().action_archive()
+
+        self.group.create_next_year_groups()
+
+        self.assertEqual(len(self._next_year_groups(active_test=False)), 1)
+
+    def test_04_last_level_of_the_plan_suggests_no_level(self):
+        self.assertEqual(self.group._get_next_year_level(), self.level_2)
+
+        closing_group = self.group.copy({"level_id": self.level_2.id})
+        self.assertFalse(closing_group._get_next_year_level(), "a group closing the plan must not suggest a next level")
+
+    def test_05_plan_without_sequence_keeps_the_same_level(self):
+        """Schools that never configured the plan must behave exactly as before."""
+        self.section.level_line_ids.unlink()
+
+        self.assertFalse(self.section._is_last_level(self.level_1))
+        self.assertEqual(self.group._get_next_year_level(), self.level_1)

academic/views/academic_section_views.xml

@@ -21,8 +21,21 @@
                     <group>
                         <field name="name" />
                         <!-- <field name="correlative_ids" widget="many2many_tags" domain="[('id', '!=', id)]"/> -->
-                        <field name="level_ids" widget="many2many_tags"/>
                     </group>
+                    <notebook>
+                        <page string="Levels" name="levels">
+                            <div class="text-muted mb-2">
+                                Drag the levels into the order the student goes through them: the
+                                re-enrollment wizard uses it to suggest the next level of each group.
+                            </div>
+                            <field name="level_line_ids">
+                                <list editable="bottom">
+                                    <field name="sequence" widget="handle"/>
+                                    <field name="level_id" options="{'no_quick_create': 1}"/>
+                                </list>
+                            </field>
+                        </page>
+                    </notebook>
                 </sheet>
             </form>
         </field>
@@ -32,11 +45,14 @@
         <field name="name">academic.section.list</field>
         <field name="model">academic.section</field>
         <field name="arch" type="xml">
-            <list editable="bottom">
+            <!-- open_form_view: the list is editable, so without it the form where the levels are ordered is unreachable -->
+            <list editable="bottom" open_form_view="True">
                 <field name="sequence" widget="handle"/>
                 <field name="name" />
                 <!-- <field name="correlative_ids" widget="many2many_tags"/> -->
-                <field name="level_ids" widget="many2many_tags" optional="show"/>
+                <!-- readonly: the ordered lines on the form are the only write path, so adding a
+                     level from here cannot silently append it at the end of the plan -->
+                <field name="level_ids" widget="many2many_tags" optional="show" readonly="1"/>
             </list>
         </field>
     </record>