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.

Sign up for GitHub

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

Jump to bottom

notifications: add cancel notification #2345

Merged
merged 1 commit into from
Sep 2, 2021
Merged

notifications: add cancel notification #2345

merged 1 commit into from
Sep 2, 2021

Conversation

jma
Copy link
Contributor

@jma jma commented Aug 30, 2021 •
edited
Loading

  • Adds a new status notifications property.
  • Cancels a requestion notification when the item is not at desk.
  • Cancels a recall, overdue and duesoon notificaition when the item is
    not on loan.
  • Adds an update mapping utility to update the mapping of an existing
    index within new properties.

Co-Authored-by: Johnny Mariéthoz [email protected]

Why are you opening this PR?

  • Which task/US does it implement?
  • Which issue does it fix?

Dependencies

My PR depends on the following rero-ils-ui"s PR(s):

  • rero/rero-ils-ui#

How to test?

  • What command should I have to run to test your PR?
  • What should I test through the UI?

Code review check list

  • Commit message template compliance.
  • Commit message without typos.
  • File names.
  • Functions names.
  • Functions docstrings.
  • Unnecessary commited files?
  • Cypress tests successful?

@jma jma force-pushed the maj-cancel-notifications branch 2 times, most recently from 54183ef to 717a239 Compare August 30, 2021 13:03
@jma jma marked this pull request as ready for review August 30, 2021 14:11
@iGormilhit iGormilhit added this to the v1.4.9 milestone Aug 31, 2021
@jma jma force-pushed the maj-cancel-notifications branch from 717a239 to ff3b7a0 Compare August 31, 2021 10:16
@github-actions github-actions bot added the f: circulation Concerns the circulation interface or backend label Aug 31, 2021
@jma jma force-pushed the maj-cancel-notifications branch 2 times, most recently from d348756 to 12b38b7 Compare August 31, 2021 13:52
@zannkukai zannkukai force-pushed the maj-cancel-notifications branch 3 times, most recently from 856e62f to 0355ded Compare August 31, 2021 18:08
iGormilhit
iGormilhit suggested changes Sep 1, 2021
View reviewed changes
Copy link

@iGormilhit iGormilhit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit message proposition:

notifications: add cancel notification

* Adds a new `status` notification property.
* Cancels a request notification when the item is not at desk.
* Cancels a recall, overdue and due soon notification when the item is
  not on loan.
* Adds an update mapping utility to update the mapping of an existing
  index within new properties.

notifications property → notification property
duesoon notificaition → due soon notification

I see that this PR takes care of the reminders (1st, 2nd, etc.). Could you verify #2348 and more globally if this PR fix any of the following issues: https://github.com/orgs/rero/projects/3?fullscreen=true&card_filter_query=label:notifications

rero_ils/modules/loans/api.py Outdated Show resolved Hide resolved
rero_ils/modules/loans/api.py Outdated Show resolved Hide resolved
rero_ils/modules/loans/api.py Show resolved Hide resolved
rero_ils/modules/loans/api.py Outdated Show resolved Hide resolved
rero_ils/modules/loans/api.py Outdated Show resolved Hide resolved
rero_ils/modules/notifications/dispatcher.py Outdated Show resolved Hide resolved
rero_ils/modules/notifications/dispatcher.py Outdated Show resolved Hide resolved
rero_ils/modules/notifications/dispatcher.py Show resolved Hide resolved
rero_ils/modules/notifications/dispatcher.py Outdated Show resolved Hide resolved
rero_ils/modules/notifications/jsonschemas/notifications/notification-v0.0.1.json Show resolved Hide resolved
@zannkukai zannkukai force-pushed the maj-cancel-notifications branch 2 times, most recently from 69d3659 to d55ee74 Compare September 1, 2021 06:47
@zannkukai zannkukai requested a review from iGormilhit September 1, 2021 06:48
@zannkukai zannkukai force-pushed the maj-cancel-notifications branch from d55ee74 to dac9638 Compare September 1, 2021 07:34
@jma @zannkukai
notifications: add cancel notification
82caf60 
* Adds a new `status` notification property.
* Cancels a request notification when the item is not at desk.
* Cancels a recall, overdue and due soon notification when the item is
  not on loan.
* Adds an update mapping utility to update the mapping of an existing
  index within new properties.

Co-Authored-by: Johnny Mariéthoz <[email protected]>
@zannkukai zannkukai force-pushed the maj-cancel-notifications branch from dac9638 to 82caf60 Compare September 1, 2021 09:14
@iGormilhit iGormilhit merged commit 9d93d25 into rero:dev Sep 2, 2021
@iGormilhit iGormilhit mentioned this pull request Sep 2, 2021
Closed
@jma jma deleted the maj-cancel-notifications branch January 13, 2022 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zannkukai zannkukai zannkukai approved these changes

@lauren-d lauren-d lauren-d approved these changes

@benerken benerken benerken approved these changes

@iGormilhit iGormilhit iGormilhit approved these changes

@rerowep rerowep Awaiting requested review from rerowep

Assignees

@jma jma

Labels
f: circulation Concerns the circulation interface or backend f: notifications f: search
Projects
None yet
Milestone
v1.4.9
Development

Successfully merging this pull request may close these issues.
6 participants
@jma @iGormilhit @zannkukai @benerken @rerowep @lauren-d