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: create data model and api #326

Merged
merged 1 commit into from
Jun 12, 2019
Merged

notifications: create data model and api #326

merged 1 commit into from
Jun 12, 2019

Conversation

BadrAly
Copy link

@BadrAly BadrAly commented Jun 5, 2019 •
edited
Loading

  • Adds notification schema, mappings and required objects
  • Integrates invenio-records 1.2.0 to have a separate table by resource

To test:

  • ./scripts/bootstrap
  • ./run-tests.sh
  • ./scripts/setup

Signed-off-by: Aly Badr aly.badr@rero.ch

@BadrAly BadrAly force-pushed the baa-#879-notifications-api branch 2 times, most recently from 2c61009 to 1e8dde0 Compare June 5, 2019 12:02
iGormilhit
iGormilhit reviewed Jun 5, 2019
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.

There's a data.json with a notif fixture, should the scripts/setup file be updated? I didn't saw it in the list of changed files. Am I missing something, or is it planned like that?

rero_ils/modules/notifications/jsonschemas/notifications/notification-v0.0.1.json Outdated Show resolved Hide resolved
@BadrAly
Copy link
Author

BadrAly commented Jun 5, 2019 •
edited
Loading

There's a data.json with a notif fixture, should the scripts/setup file be updated? I didn't saw it in the list of changed files. Am I missing something, or is it planned like that?

This file tests/data/data.json is used by python unittest and not by scripts/setup.
In same time, I will ask @pronguen if he needs to have notification fixtures for the frontend

@iGormilhit
Copy link

@BadrAly Oh, sorry, I wasn't paying enough attention! 🙂

@iGormilhit iGormilhit self-requested a review June 5, 2019 15:25
@BadrAly BadrAly force-pushed the baa-#879-notifications-api branch from 1e8dde0 to b03816d Compare June 6, 2019 06:27
jma
jma requested changes Jun 11, 2019
View reviewed changes
Pipfile Outdated Show resolved Hide resolved
Pipfile.lock Outdated Show resolved Hide resolved
Pipfile.lock Outdated Show resolved Hide resolved
rero_ils/modules/notifications/jsonschemas/notifications/notification-v0.0.1.json Outdated Show resolved Hide resolved
rero_ils/modules/notifications/mappings/v6/notifications/notification-v0.0.1.json Show resolved Hide resolved
rero_ils/modules/notifications/views.py Show resolved Hide resolved
tests/api/test_notifications_rest.py Outdated Show resolved Hide resolved
tests/data/data.json Outdated Show resolved Hide resolved
tests/fixtures/circulation.py Outdated Show resolved Hide resolved
tests/ui/notifications/test_notifications_api.py Outdated Show resolved Hide resolved
@BadrAly BadrAly force-pushed the baa-#879-notifications-api branch 2 times, most recently from 8aae098 to 0c738a9 Compare June 12, 2019 07:07
@BadrAly BadrAly requested a review from jma June 12, 2019 08:03
jma
jma requested changes Jun 12, 2019
View reviewed changes
tests/api/test_loans_rest.py Outdated Show resolved Hide resolved
tests/fixtures/circulation.py Outdated Show resolved Hide resolved
tests/fixtures/metadata.py Outdated Show resolved Hide resolved
tests/ui/notifications/test_notifications_api.py Outdated Show resolved Hide resolved
@BadrAly BadrAly force-pushed the baa-#879-notifications-api branch from 0c738a9 to 8cc7b73 Compare June 12, 2019 11:12
notifications: create resource data model and api
78d7507 
* Adds notification schema, mappings and required objects
* Integrate invenio-records 1.2.1 to have a separate table by resource

Signed-off-by: Aly Badr <aly.badr@rero.ch>
@BadrAly BadrAly force-pushed the baa-#879-notifications-api branch from 8cc7b73 to 78d7507 Compare June 12, 2019 11:13
@BadrAly BadrAly merged commit e97483c into rero:US341 Jun 12, 2019
@BadrAly BadrAly deleted the baa-#879-notifications-api branch July 1, 2019 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iGormilhit iGormilhit iGormilhit approved these changes

@jma jma jma approved these changes

@rerowep rerowep Awaiting requested review from rerowep

@Garfield-fr Garfield-fr Awaiting requested review from Garfield-fr

@reropag reropag Awaiting requested review from reropag

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@BadrAly @iGormilhit @jma