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

notifications: patron url #1029

Merged
merged 1 commit into from
Jun 19, 2020
Merged

Conversation

rerowep
Copy link
Contributor

@rerowep rerowep commented Jun 7, 2020

  • Corrects patron url in notifications. RERO_ILS_APP_URL is used to
    construct the patron url. closes Link to the patron profile not adapted to the concerned instance in the notification message. #802
  • Adds comand line interface for notifications. Process of notifications
    can now be started with invenio run notifications process.
  • Uses standart JSONSCHEMAS_ENDPOINT = '/schemas'.
  • Uses JSONSCHEMAS_REPLACE_REFS = True to resolve $ref before serving a
    schema.
  • Deletes all unnecessary $schema in fixtures.

Why are you opening this PR?

How to test?

  • Run setup and verify the patron urls are correct in the send emails.

Code review check list

  • Commit message template compliance.
  • Commit message without typos.
  • File names.
  • Functions names.
  • Functions docstrings.
  • Unnecessary commited files?
  • Extracted translations?

@rerowep rerowep self-assigned this Jun 7, 2020
@rerowep rerowep force-pushed the wep-#1537-fix-patron-link branch from db0ef6b to bbe3871 Compare June 7, 2020 22:02
@iGormilhit iGormilhit self-requested a review June 8, 2020 04:56
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.

My suggestion for the commit message:

notifications: fix patron URL construction

The patron profile URL in the notification messages should be adapted to the running instance.

* Corrects patron URL. `RERO_ILS_APP_BASE_URL` is used to construct the patron URL. Closes #802.
* Adds command line interface for notifications. Process of notifications can now be started with `invenio run notifications process`.

@rerowep rerowep force-pushed the wep-#1537-fix-patron-link branch from bbe3871 to 838b707 Compare June 8, 2020 05:46
@rerowep rerowep marked this pull request as draft June 8, 2020 11:22
@rerowep rerowep requested a review from iGormilhit June 8, 2020 11:22
@rerowep rerowep force-pushed the wep-#1537-fix-patron-link branch 3 times, most recently from 8943b36 to b3d9e34 Compare June 9, 2020 08:54
@rerowep rerowep marked this pull request as ready for review June 9, 2020 08:56
@rerowep rerowep requested review from BadrAly and lauren-d June 9, 2020 08:56
@rerowep rerowep force-pushed the wep-#1537-fix-patron-link branch from b3d9e34 to b27fa78 Compare June 9, 2020 09:03
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:

notifications: adapt patron url to the instance

The patron URL displayed in the notifications sent to to the patron
should be automatically changed according to the deployed instance.

* Corrects patron URL in notifications. `RERO_ILS_APP_URL` is used to
  construct the patron URL. Closes #802
* Adds command line interface for notifications. Process of
  notifications can now be started with `invenio run notifications
  process`.
* Uses standart JSONSCHEMAS_ENDPOINT = '/schemas'.
* Uses JSONSCHEMAS_REPLACE_REFS = True to resolve $ref before serving a
  schema.
* Deletes all unnecessary `$schema` in fixtures.

@rerowep rerowep marked this pull request as draft June 11, 2020 06:21
@rerowep rerowep marked this pull request as ready for review June 11, 2020 06:22
@rerowep rerowep force-pushed the wep-#1537-fix-patron-link branch from b27fa78 to 60c49d9 Compare June 11, 2020 07:13
@rerowep rerowep requested a review from iGormilhit June 11, 2020 07:13
@rerowep rerowep requested review from zannkukai and jma June 15, 2020 12:16
@rerowep rerowep force-pushed the wep-#1537-fix-patron-link branch from 60c49d9 to 01394d6 Compare June 16, 2020 05:38
Copy link
Contributor

@zannkukai zannkukai left a comment

Choose a reason for hiding this comment

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

Great job 👍

rero_ils/modules/notifications/api.py Show resolved Hide resolved
Copy link
Contributor

@jma jma left a comment

Choose a reason for hiding this comment

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

Good news to do this. Can we split the JSONSchema document into several files? Languages, etc.

@rerowep rerowep force-pushed the wep-#1537-fix-patron-link branch from 01394d6 to 9de1675 Compare June 17, 2020 12:53
@rerowep
Copy link
Contributor Author

rerowep commented Jun 17, 2020

Good news to do this. Can we split the JSONSchema document into several files? Languages, etc.

Should be interesting to test.

@rerowep rerowep requested review from jma and zannkukai June 18, 2020 10:48
@rerowep rerowep requested a review from iGormilhit June 18, 2020 10:48
rero_ils/modules/notifications/api.py Outdated Show resolved Hide resolved
@rerowep rerowep force-pushed the wep-#1537-fix-patron-link branch from 9de1675 to 745a296 Compare June 18, 2020 11:53
@rerowep rerowep requested a review from BadrAly June 18, 2020 11:55
The patron URL displayed in the notifications sent to to the patron
should be automatically changed according to the deployed instance.

* Corrects patron URL in notifications. `RERO_ILS_APP_URL` is used to
  construct the patron URL. Closes rero#802
* Adds command line interface for notifications. Process of
  notifications can now be started with `invenio run notifications
  process`.
* Uses standart JSONSCHEMAS_ENDPOINT = '/schemas'.
* Uses JSONSCHEMAS_REPLACE_REFS = True to resolve $ref before serving a
  schema.
* Deletes all unnecessary `$schema` in fixtures.
* Deletes `document-minimal-v0.0.1.json` schema file.

Co-Authored-by: Peter Weber <peter.weber@rero.ch>
@rerowep rerowep force-pushed the wep-#1537-fix-patron-link branch from 745a296 to 31968af Compare June 18, 2020 12:35
@rerowep rerowep merged commit adbf9ba into rero:dev Jun 19, 2020
@rerowep rerowep deleted the wep-#1537-fix-patron-link branch June 19, 2020 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Link to the patron profile not adapted to the concerned instance in the notification message.
6 participants