From 498eb0cfed02a650cd9221af9587e13aab634704 Mon Sep 17 00:00:00 2001 From: Damien Abeloos Date: Wed, 16 Dec 2020 15:23:59 +0000 Subject: [PATCH] [IMP] event: improve the event form editor and update the tour * In the Communication tab, prevent inserting multiple duplicate lines upon switching template. - Before, when the user was switching between templates, the lines introduced with the last template were not removed, and the amount of lines only kept increasing. - Now, only the lines that are linked to a registration are kept * In the Tickets tab, prevent inserting multiple duplicate lines upon switching template (Ticketing) if there was already tickets linked to a registration. - Same problem as in the Communication tab, multiple lines could be introduced. - Now, only the lines that are linked to a registration are kept * Add 2 tests (compute mails and tickets) to ensure those behaviors are properly maintained in future updates. As the issue occured only in a non-saved Form when switching event_type templates, the tests are a little low level (since we have to check the computed results directly in the Form). * Remove the last 2 steps of the event_tour : - They are a bit out of scope and they break the rythm (better to end on a high note) Task ID : 2410217 PR : https://github.com/odoo/odoo/pull/63370 --- addons/event/models/event_event.py | 26 +-- .../event/static/src/js/tours/event_tour.js | 14 -- addons/event/tests/test_event_internals.py | 190 ++++++++++++++---- 3 files changed, 159 insertions(+), 71 deletions(-) diff --git a/addons/event/models/event_event.py b/addons/event/models/event_event.py index c838c1c0e50..ca113423f2e 100644 --- a/addons/event/models/event_event.py +++ b/addons/event/models/event_event.py @@ -4,7 +4,7 @@ import logging import pytz -from odoo import _, api, fields, models +from odoo import _, api, Command, fields, models from odoo.addons.base.models.res_partner import _tz_get from odoo.tools import format_datetime from odoo.exceptions import ValidationError @@ -397,11 +397,13 @@ class EventEvent(models.Model): continue # lines to keep: those with already sent emails or registrations - mails_toremove = event._origin.event_mail_ids.filtered(lambda mail: not mail.mail_sent and not(mail.mail_registration_ids)) - command = [(3, mail.id) for mail in mails_toremove] + mails_to_remove = event.event_mail_ids.filtered( + lambda mail: not(mail._origin.mail_sent or mail._origin.mail_registration_ids) + ) + command = [Command.unlink(mail.id) for mail in mails_to_remove] if event.event_type_id.use_mail_schedule: command += [ - (0, 0, { + Command.create({ attribute_name: line[attribute_name] if not isinstance(line[attribute_name], models.BaseModel) else line[attribute_name].id for attribute_name in self.env['event.type.mail']._get_event_mail_fields_whitelist() }) for line in event.event_type_id.event_type_mail_ids @@ -434,27 +436,17 @@ class EventEvent(models.Model): Note that updating event_ticket_ids triggers _compute_start_sale_date (start_sale_date computation) so ensure result to avoid cache miss. """ - if self.ids or self._origin.ids: - # lines to keep: those with already sent emails or registrations - tickets_tokeep_ids = self.env['event.registration'].search( - [('event_id', 'in', self.ids or self._origin.ids)] - ).event_ticket_id.ids - else: - tickets_tokeep_ids = [] for event in self: if not event.event_type_id and not event.event_ticket_ids: event.event_ticket_ids = False continue # lines to keep: those with existing registrations - if tickets_tokeep_ids: - tickets_toremove = event._origin.event_ticket_ids.filtered(lambda ticket: ticket.id not in tickets_tokeep_ids) - command = [(3, ticket.id) for ticket in tickets_toremove] - else: - command = [(5, 0)] + tickets_to_remove = event.event_ticket_ids.filtered(lambda ticket: not ticket._origin.registration_ids) + command = [Command.unlink(ticket.id) for ticket in tickets_to_remove] if event.event_type_id.use_ticket: command += [ - (0, 0, { + Command.create({ attribute_name: line[attribute_name] if not isinstance(line[attribute_name], models.BaseModel) else line[attribute_name].id for attribute_name in self.env['event.type.ticket']._get_event_ticket_fields_whitelist() }) for line in event.event_type_id.event_type_ticket_ids diff --git a/addons/event/static/src/js/tours/event_tour.js b/addons/event/static/src/js/tours/event_tour.js index 192d243d924..d86f6330a0e 100644 --- a/addons/event/static/src/js/tours/event_tour.js +++ b/addons/event/static/src/js/tours/event_tour.js @@ -74,20 +74,6 @@ tour.register('event_tour', { content: _t("Use the breadcrumbs to go back to your kanban overview."), position: 'bottom', run: 'click', -}, { - trigger: '.o_event_kanban_view div.o_quick_create_folded', - content: _t("This pipeline can be customized on the fly to fit your organizational needs. For example, let's create a new stage."), - position: 'bottom', - run: function (actions) { - actions.click(); - $('div.o_kanban_header input[type="text"]').val('New Stage'); - }, -}, { - trigger: '.o_event_kanban_view button.o_kanban_add', - content: _t("Click add to create a new stage."), - position: 'bottom', - width: 200, - run: 'click', }].filter(Boolean)); }); diff --git a/addons/event/tests/test_event_internals.py b/addons/event/tests/test_event_internals.py index 5f4c7753e3d..8f6d25af0e8 100644 --- a/addons/event/tests/test_event_internals.py +++ b/addons/event/tests/test_event_internals.py @@ -4,10 +4,11 @@ from datetime import date, datetime, timedelta from unittest.mock import patch +from odoo import Command from odoo.addons.event.tests.common import TestEventCommon from odoo import exceptions from odoo.fields import Datetime as FieldsDatetime, Date as FieldsDate -from odoo.tests.common import users +from odoo.tests.common import users, Form from odoo.tools import mute_logger @@ -72,9 +73,7 @@ class TestEventData(TestEventCommon): @users('user_eventmanager') @mute_logger('odoo.models.unlink') def test_event_configuration_from_type(self): - """ Test data computation of event coming from its event.type template. - Some one2many notably are duplicated from type configuration and some - advanced testing is required, notably mail schedulers. """ + """ Test data computation of event coming from its event.type template. """ self.assertEqual(self.env.user.tz, 'Europe/Brussels') # ------------------------------------------------------------ @@ -145,46 +144,157 @@ class TestEventData(TestEventCommon): event_ticket1 = event.event_ticket_ids[0] self.assertEqual(event_ticket1.name, 'TestRegistration') - # ------------------------------------------------------------ - # RESET TEST - # ------------------------------------------------------------ - - # link registration to ticket - registration.write({'event_ticket_id': event_ticket1.id}) - self.assertEqual(registration.event_ticket_id, event_ticket1) - - # change template to a void one for mails -> reset event lines that are void - # change template to a one with other tickets -> keep line linked to a registration - event_type.write({ + @users('user_eventmanager') + def test_event_configuration_mails_from_type(self): + """ Test data computation (related to mails) of event coming from its event.type template. + This test uses pretty low level Form data checks, as manipulations in a non-saved Form are + required to highlight an undesired behavior when switching event_type templates : + event_mail_ids not linked to a registration were generated and kept when switching between + different templates in the Form, which could rapidly lead to a substantial amount of + undesired lines. """ + # setup test records + event_type_default = self.env['event.type'].create({ + 'name': 'Type Default', 'use_mail_schedule': False, - 'event_type_mail_ids': [(5, 0)], - 'event_type_ticket_ids': [(5, 0), - (0, 0, {'name': 'Registration1'}), - (0, 0, {'name': 'Registration2'})], + 'auto_confirm': True }) - event._compute_event_ticket_ids() - event._compute_event_mail_ids() - self.assertEqual(event.event_mail_ids, self.env['event.mail']) - self.assertEqual(len(event.event_ticket_ids), 3) - self.assertEqual( - set(event.mapped('event_ticket_ids.name')), - set(['TestRegistration', 'Registration1', 'Registration2']) - ) - # registration loose its ticket - self.assertEqual(registration.event_ticket_id, event_ticket1) - - # change template to a one with different mails -> reset event - event_type.write({ + event_type_mails = self.env['event.type'].create({ + 'name': 'Type Mails', + 'auto_confirm': False + }) + event_type_mails.write({ 'use_mail_schedule': True, - 'event_type_mail_ids': [(5, 0), (0, 0, { - 'interval_nbr': 3, 'interval_unit': 'days', 'interval_type': 'after_event', - 'template_id': self.env['ir.model.data'].xmlid_to_res_id('event.event_reminder')})] + 'event_type_mail_ids': [ + Command.clear(), + Command.create({ + 'notification_type': 'mail', + 'interval_nbr': 77, + 'interval_unit': 'days', + 'interval_type': 'after_event', + 'template_id': self.env.ref('event.event_reminder').id, + }) + ] }) - event._compute_event_ticket_ids() - event._compute_event_mail_ids() - self.assertEqual(len(event.event_mail_ids), 1) - self.assertEqual(event.event_mail_ids.interval_nbr, 3) - self.assertEqual(event.event_mail_ids.interval_type, 'after_event') + event = self.env['event.event'].create({ + 'name': 'Event', + 'date_begin': FieldsDatetime.to_string(datetime.today() + timedelta(days=1)), + 'date_end': FieldsDatetime.to_string(datetime.today() + timedelta(days=15)), + 'event_type_id': event_type_default.id + }) + event.write({ + 'event_mail_ids': [ + Command.clear(), + Command.create({ + 'notification_type': 'mail', + 'interval_unit': 'now', + 'interval_type': 'after_sub', + 'template_id': self.env.ref('event.event_subscription').id, + }) + ] + }) + mail = event.event_mail_ids[0] + registration = self._create_registrations(event, 1) + self.assertEqual(registration.state, 'open') # event auto confirms + # verify that mail is linked to the registration + self.assertEqual( + set(mail.mapped('mail_registration_ids.registration_id.id')), + set([registration.id]) + ) + # start test scenario + event_form = Form(event) + # verify that mail is linked to the event in the form + self.assertEqual( + set(map(lambda m: m.get('id', None), event_form.event_mail_ids._records)), + set([mail.id]) + ) + # switch to an event_type with a mail template which should be computed + event_form.event_type_id = event_type_mails + # verify that 2 mails were computed + self.assertEqual(len(event_form.event_mail_ids._records), 2) + # verify that the mail linked to the registration was kept + self.assertTrue(filter(lambda m: m.get('id', None) == mail.id, event_form.event_mail_ids._records)) + # since the other computed event.mail is to be created from an event.type.mail template, + # verify that its attributes are the correct ones + computed_mail = next(filter(lambda m: m.get('id', None) != mail.id, event_form.event_mail_ids._records), {}) + self.assertEqual(computed_mail.get('interval_nbr', None), 77) + self.assertEqual(computed_mail.get('interval_unit', None), 'days') + self.assertEqual(computed_mail.get('interval_type', None), 'after_event') + # switch back to an event type without a mail template + event_form.event_type_id = event_type_default + # verify that the mail linked to the registration was kept, and the other removed + self.assertEqual( + set(map(lambda m: m.get('id', None), event_form.event_mail_ids._records)), + set([mail.id]) + ) + + @users('user_eventmanager') + def test_event_configuration_tickets_from_type(self): + """ Test data computation (related to tickets) of event coming from its event.type template. + This test uses pretty low level Form data checks, as manipulations in a non-saved Form are + required to highlight an undesired behavior when switching event_type templates : + event_ticket_ids not linked to a registration were generated and kept when switching between + different templates in the Form, which could rapidly lead to a substantial amount of + undesired lines. """ + # setup test records + event_type_default = self.env['event.type'].create({ + 'name': 'Type Default', + 'use_ticket': False, + 'auto_confirm': True + }) + event_type_tickets = self.env['event.type'].create({ + 'name': 'Type Tickets', + 'auto_confirm': False + }) + event_type_tickets.write({ + 'use_ticket': True, + 'event_type_ticket_ids': [ + Command.clear(), + Command.create({ + 'name': 'Default Ticket', + 'seats_max': 10, + }) + ] + }) + event = self.env['event.event'].create({ + 'name': 'Event', + 'date_begin': FieldsDatetime.to_string(datetime.today() + timedelta(days=1)), + 'date_end': FieldsDatetime.to_string(datetime.today() + timedelta(days=15)), + 'event_type_id': event_type_default.id + }) + event.write({ + 'event_ticket_ids': [ + Command.clear(), + Command.create({ + 'name': 'Registration Ticket', + 'seats_max': 10, + }) + ] + }) + ticket = event.event_ticket_ids[0] + registration = self._create_registrations(event, 1) + # link the ticket to the registration + registration.write({'event_ticket_id': ticket.id}) + # start test scenario + event_form = Form(event) + # verify that the ticket is linked to the event in the form + self.assertEqual( + set(map(lambda m: m.get('name', None), event_form.event_ticket_ids._records)), + set(['Registration Ticket']) + ) + # switch to an event_type with a ticket template which should be computed + event_form.event_type_id = event_type_tickets + # verify that both tickets are computed + self.assertEqual( + set(map(lambda m: m.get('name', None), event_form.event_ticket_ids._records)), + set(['Registration Ticket', 'Default Ticket']) + ) + # switch back to an event_type without default tickets + event_form.event_type_id = event_type_default + # verify that the ticket linked to the registration was kept, and the other removed + self.assertEqual( + set(map(lambda m: m.get('name', None), event_form.event_ticket_ids._records)), + set(['Registration Ticket']) + ) @users('user_eventmanager') def test_event_registrable(self):