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

holdings: add optional fields for holdings display #1244

Merged
merged 1 commit into from
Oct 14, 2020

Conversation

BadrAly
Copy link

@BadrAly BadrAly commented Sep 24, 2020

With this commit, new optional fields are allowed for
the holdings of type serial. The fields are:

  • enumerationAndChronology
  • supplementaryContent
  • index
  • missing_issues
  • notes

The new field second_call_number is available for
all types of holdings records

Item and holdings call numbers have no minimum character constraint.

Issues with status deleted are not displayed in the public interface.

Co-Authored-by: Aly Badr aly.badr@rero.ch

Why are you opening this PR?

https://tree.taiga.io/project/rero21-reroils/taskboard/sprint-35-106?kanban-status=1224894

How to test?

bootstrap + setup
To display a serial holdings with optional field, display the following document in the public interface:
~/global/search/documents?q=R003826164&page=1&size=10

Code review check list

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

@BadrAly BadrAly self-assigned this Sep 24, 2020
@BadrAly BadrAly force-pushed the baa-add-holdings-fields branch 12 times, most recently from 1766c9e to 0169013 Compare October 8, 2020 11:14
@BadrAly BadrAly marked this pull request as ready for review October 8, 2020 11:36
@BadrAly BadrAly added this to the v0.13.0 milestone Oct 8, 2020
@BadrAly BadrAly force-pushed the baa-add-holdings-fields branch 7 times, most recently from 7fba716 to 8806950 Compare October 13, 2020 10:32
Copy link
Contributor

@pronguen pronguen left a comment

Choose a reason for hiding this comment

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

  • All the notes should be displayed in the professional interface, with a dedicated label (currently: only general note). In public interface, it is correct: only display the general note.
  • Why is the "Duplicate" button not aligned with the others?
    image
  • The line breaks inside the notes should be displayed as line breaks in pro/public interface. This can also be realised in US 1724

@lauren-d
Copy link
Contributor

  • In the professional interface, the counter is not display correctly when there are some hidden issue.

Capture d’écran 2020-10-14 à 08 41 48

@Garfield-fr
Copy link
Contributor

  • In the professional interface, the counter is not display correctly when there are some hidden issue.
Capture d’écran 2020-10-14 à 08 41 48

This problem is present because the translation of the string is missing.

@JoelleDosimont
Copy link
Contributor

image

@JoelleDosimont
Copy link
Contributor

Copy link
Contributor

@pronguen pronguen left a comment

Choose a reason for hiding this comment

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

  • holdings wihtout items should be display also in the public view (there is the collection description)
    • Ideally without the expand button and without "No items received"
  • items/issues that have status deleted should not be displayed in public interface (an issue can be opened for that...)

@BadrAly BadrAly force-pushed the baa-add-holdings-fields branch 2 times, most recently from 1132fac to 709f62b Compare October 14, 2020 12:36
With this commit, new optional fields are allowed for
the holdings of type serial. The fields are:
- enumerationAndChronology
- supplementaryContent
- index
- missing_issues
- notes

The new field second_call_number is available for
all types of holdings records

Item and holdings call numbers have no minimum character constraint.

Issues with status deleted are not displayed in the public interface.

* Closes rero#1284
* Removes the condition to have an item second call number only
if first call number exist.
* Adapts tests and fixtures accordingly.
* Adapts the public interface to display the new fields.

Co-Authored-by: Aly Badr <aly.badr@rero.ch>
@BadrAly BadrAly merged commit 8b88dd7 into rero:dev Oct 14, 2020
@BadrAly BadrAly deleted the baa-add-holdings-fields branch November 5, 2020 13:40
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.

A call number (1st and 2nd) should not have a minimum caracter constraint.
7 participants