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

documents: improve editor #206

Merged
merged 1 commit into from
May 26, 2020
Merged

documents: improve editor #206

merged 1 commit into from
May 26, 2020

Conversation

sebdeleze
Copy link
Contributor

@sebdeleze sebdeleze commented May 1, 2020

documents: improve editor

  • Configures JSON schema for improving document editor.
  • Adds constraints and validations for certain fields.
  • Adds translations for bibframe types.
  • Adds a schema for files in marshmallow.
  • Renames from "restricted" to "restriction" the object giving the restriction information of a file.
  • Removes "restriction" object when a file is loaded in marshmallow.
  • Fixes issue on file restriction when organisation is not resolved.
  • Completes document types items list.
  • Hierarchize form options based on a level property.
  • Re-organize items for Classification field.
  • Closes Improves document editor, as it displays all the field now and it cannot be saved. sonar-ui#63.

Co-Authored-by: Sébastien Délèze sebastien.deleze@rero.ch

How to test

  1. Login as an administrator.
  2. Go to Admin -> Records -> Documents.
  3. Click Add button.
  4. Fill the form.
  5. Save document.
  6. Edit document and change some fields.
  7. Save document.
  8. Delete document.

NOTE: The aim of this PR is to confirm that documents can be created or updated with the form. Maybe some placeholders or validations are missing, we can add them later.

@sebdeleze sebdeleze marked this pull request as ready for review May 1, 2020 08:33
@sebdeleze sebdeleze requested a review from zannkukai May 6, 2020 09:51
Comment on lines 1256 to 1374
"propertiesOrder": [
"documentType",
"title",
"organisation",
"classification",
"language",
"abstracts",
"subjects",
"identifiedBy",
"contribution",
"dissertation",
"editionStatement",
"otherEdition",
"provisionActivity",
"partOf",
"specificCollections",
"extent",
"formats",
"notes",
"series",
"additionalMaterials",
"contentNote",
"otherMaterialCharacteristics",
"usageAndAccessPolicy"
]

Choose a reason for hiding this comment

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

maybe better to place propertiesOrder tag just next to required field ?

Copy link
Contributor Author

@sebdeleze sebdeleze May 11, 2020

Choose a reason for hiding this comment

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

I chose this order:

  1. properties
  2. propertiesOrder
  3. required

The same should be applied for all the properties. What is you opinion about that?

If you agree, I will propose that in https://github.com/rero/developer-resources/blob/master/guidelines/json-schemas.md

Choose a reason for hiding this comment

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

It's not a problem for me if all project schemas follow the same pattern.
I think it's more easy to find "required" and "propertiesOrder" at the top of the file (easier to find). That's my personal opinion, not a guideline :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Accordingly to the freshly approved section https://github.com/rero/developer-resources/blob/master/guidelines/json-schemas.md#order, I did the changes.

Copy link

@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.

Seems OK for me. Just some not blocking comments

@sebdeleze sebdeleze marked this pull request as draft May 7, 2020 06:05
@sebdeleze
Copy link
Contributor Author

sebdeleze commented May 10, 2020

I encountered some potential issues within the editor:

  • On field Identifier --> Source, I got ExpressionChangedAfterItHasBeenCheckedError error when validation with expression is added.
  • Field of type const should not be displayed in the editor. Currently, a line with only the trash icon is displayed.
  • With oneOf fields, validation is not applied correctly (field Classification).
  • With oneOf fields, when option 2 is selected, on page reload the second option is not selected (field Classification).
  • When a field validation is depending on other field, validation is not applied on depending field change (Contribution --> Preferred name).

@jma Could we take a look and determine if these points are due to misconfiguration or editor issue.

@sebdeleze
Copy link
Contributor Author

@pronguen In CDU.xlsx file on switch drive, two fields contains the same ID 61: Medicine and Health. Medicine is the parent of Health. Is this a mistake?

@pronguen
Copy link
Contributor

@sebastiendeleze I think it is not a mistake. We should just take these ID as they are...

@sebdeleze
Copy link
Contributor Author

@zannkukai Sorry I had to do some changes, could you take a look again, please?

@sebdeleze sebdeleze marked this pull request as ready for review May 11, 2020 14:11
@sebdeleze sebdeleze requested a review from zannkukai May 11, 2020 14:11
for option in value['form']['options']:
if option.get('level'):
option['label'] = "{spaces} {label}".format(
spaces='-' * 2 * option['level'],

Choose a reason for hiding this comment

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

did you tried " " instead of a dash ? If it works it should be more readable (my opinion)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I completely agree, but spaces are trimmed somewhere in ngx-formly..

@iGormilhit iGormilhit self-requested a review May 13, 2020 12:00
* Configures JSON schema for improving document editor.
* Adds constraints and validations for certain fields.
* Adds translations for bibframe types.
* Adds a schema for files in marshmallow.
* Renames from "restricted" to "restriction" the object giving the restriction information of a file.
* Removes "restriction" object when a file is loaded in marshmallow.
* Fixes issue on file restriction when organisation is not resolved.
* Completes document types items list.
* Hierarchize form options based on a `level` property.
* Re-organize items for Classification field.
* Closes rero/sonar-ui#63.

Co-Authored-by: Sébastien Délèze <sebastien.deleze@rero.ch>
@sebdeleze sebdeleze merged commit b7cfaaf into rero:dev May 26, 2020
@sebdeleze sebdeleze deleted the sed-doc-editor branch May 26, 2020 05:44
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.

Improves document editor, as it displays all the field now and it cannot be saved.
4 participants