Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overdue notifications: inconsistency between reminder numbers #2390

Closed
pronguen opened this issue Sep 16, 2021 · 3 comments · Fixed by #2393
Closed

Overdue notifications: inconsistency between reminder numbers #2390

pronguen opened this issue Sep 16, 2021 · 3 comments · Fixed by #2393
Assignees
Labels
bug Breaks something but is not blocking f: notifications p-High High priority (to be solved in the 2-3 next months)

Comments

@pronguen
Copy link
Contributor

Describe the bug

For a 2nd reminder, the reminder number is not the same in the e-mail object and in the note (e-mail body).

To Reproduce

Not clear. Maybe the problem happens with all 2nd reminders. See example.

Expected behavior

The reminder number is the same in the two places.

Context

  • server: bib.rero.ch
  • version: v1.4.11

Example

Sujet : 1er rappel
--
De : "[email protected]" <[email protected]>
Date : 16.09.2021, 07:34

Patron pid: 64434
Street number
2800 DELEMONT

Chère lectrice, cher lecteur,

La durée du prêt des documents mentionnés ci-dessous est échue :

Titre : Les runes : [la magie de leurs pouvoirs] / Jean-Paul Ronecker
Echéance : 24.08.2021
Note : deuxième rappel

Vous pouvez consulter votre compte et prolonger la durée de prêt de vos documents à l'adresse : [...]

Avec nos compliments

Bibliothèque municipale de Delémont - section adultes
Rue de l'Hôpital 47, 2800 Delémont
@pronguen pronguen added f: notifications bug Breaks something but is not blocking p-High High priority (to be solved in the 2-3 next months) labels Sep 16, 2021
@zannkukai
Copy link
Contributor

@pronguen
This is "normal" at this time because the subject is hardcoded into the template (https://github.com/rero/rero-ils/blob/dev/rero_ils/modules/notifications/templates/email/overdue/fre.txt#L1).

In the notification refactoring PR, I simply use "Rappel" subject (removing the numbering).

As OVERDUE messages could be aggregated, we could send 1 reminder for itemA, second reminder for itemB and fourth reminder for itemC in the same message. So it's impossible to determine the subject based on the reminder counter. This is why I choose "Rappel" simple string as subject.

An other solution could be to aggregated reminders notification adding the reminder_counter into the aggregation key.
In this case : if we need to process 3 notifs (1st_reminder for itemA, 1st_reminder for itemB, 4th_reminder for itemC), the dispatcher will send 2 mail --> 1 for both 1st_reminder, 1 for the 4th_reminder (the user could receive several emails for overdue notifications in the same day)

@zannkukai zannkukai linked a pull request Sep 20, 2021 that will close this issue
7 tasks
@zannkukai zannkukai self-assigned this Sep 20, 2021
@pronguen
Copy link
Contributor Author

@zannkukai Happy to know that it is as expected!

Indeed, I knew the problem of the overdue grouping. Maybe the aggregation only within the same overdue number is better, if the library has e-mail filters based on the e-mail object.

But for now, I am ok with both solutions. So let's follow the easiest way.

@zannkukai
Copy link
Contributor

@pronguen
The reminders workflow allows to choose which template to use for which reminders.
So it's possible (and even better ?) to create a new template files in the system for each overdue reminder.
At UCLouvain, we will create separate template files for last reminders --> sent by mail with special juridic content. The subject of this mail should be "last reminder before jail !"

zannkukai added a commit to zannkukai/rero-ils that referenced this issue Oct 7, 2021
This refactoring purpose is to generalize the notification system in order
to manage every kind of notification. The `Notification` resource
structure is heavy linked to circulation operation (we need to link the
notification with a loan).

This commit implements a specific class implementation for each type of
notification, with a subclass that provides relevant information to
process and dispatch the notification.

Closes rero#2373.
Closes rero#2390.
Closes rero#2410.

Co-Authored-by: Renaud Michotte <[email protected]>
zannkukai added a commit to zannkukai/rero-ils that referenced this issue Oct 11, 2021
This refactoring purpose is to generalize the notification system in order
to manage every kind of notification. The `Notification` resource
structure is heavy linked to circulation operation (we need to link the
notification with a loan).

This commit implements a specific class implementation for each type of
notification, with a subclass that provides relevant information to
process and dispatch the notification.

Closes rero#2373.
Closes rero#2390.
Closes rero#2410.

Co-Authored-by: Renaud Michotte <[email protected]>
zannkukai added a commit to zannkukai/rero-ils that referenced this issue Oct 12, 2021
This refactoring purpose is to generalize the notification system in order
to manage every kind of notification. The `Notification` resource
structure is heavy linked to circulation operation (we need to link the
notification with a loan).

This commit implements a specific class implementation for each type of
notification, with a subclass that provides relevant information to
process and dispatch the notification.

Closes rero#2373.
Closes rero#2390.
Closes rero#2410.

Co-Authored-by: Renaud Michotte <[email protected]>
zannkukai added a commit to zannkukai/rero-ils that referenced this issue Oct 12, 2021
This refactoring purpose is to generalize the notification system in order
to manage every kind of notification. The `Notification` resource
structure is heavy linked to circulation operation (we need to link the
notification with a loan).

This commit implements a specific class implementation for each type of
notification, with a subclass that provides relevant information to
process and dispatch the notification.

Closes rero#2373.
Closes rero#2390.
Closes rero#2410.

Co-Authored-by: Renaud Michotte <[email protected]>
zannkukai added a commit to zannkukai/rero-ils that referenced this issue Oct 12, 2021
This refactoring purpose is to generalize the notification system in order
to manage every kind of notification. The `Notification` resource
structure is heavy linked to circulation operation (we need to link the
notification with a loan).

This commit implements a specific class implementation for each type of
notification, with a subclass that provides relevant information to
process and dispatch the notification.

Closes rero#2373.
Closes rero#2390.
Closes rero#2410.

Co-Authored-by: Renaud Michotte <[email protected]>
zannkukai added a commit to zannkukai/rero-ils that referenced this issue Oct 13, 2021
This refactoring purpose is to generalize the notification system in order
to manage every kind of notification. The `Notification` resource
structure is heavy linked to circulation operation (we need to link the
notification with a loan).

This commit implements a specific class implementation for each type of
notification, with a subclass that provides relevant information to
process and dispatch the notification.

Closes rero#2373.
Closes rero#2390.
Closes rero#2410.

Co-Authored-by: Renaud Michotte <[email protected]>
zannkukai added a commit that referenced this issue Oct 13, 2021
This refactoring purpose is to generalize the notification system in order
to manage every kind of notification. The `Notification` resource
structure is heavy linked to circulation operation (we need to link the
notification with a loan).

This commit implements a specific class implementation for each type of
notification, with a subclass that provides relevant information to
process and dispatch the notification.

Closes #2373.
Closes #2390.
Closes #2410.

Co-Authored-by: Renaud Michotte <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Breaks something but is not blocking f: notifications p-High High priority (to be solved in the 2-3 next months)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants