Skip to content

support for defining relationship lookups in the submission forms#64

Merged
tdonohue merged 2 commits intoDSpace:masterfrom
atmire:submission-form-entity
Jul 17, 2019
Merged

support for defining relationship lookups in the submission forms#64
tdonohue merged 2 commits intoDSpace:masterfrom
atmire:submission-form-entity

Conversation

@benbosman
Copy link
Copy Markdown
Member

Example of a closed and open relationship in the submission forms
This has been discussed in the entities meeting, and is documented at https://docs.google.com/document/d/1X0XsppZYOtPtbmq7yXwmu7FbMAfLxxOCONbw0_rl7jY/edit#heading=h.5bu9shx0j942 and https://docs.google.com/document/d/1X0XsppZYOtPtbmq7yXwmu7FbMAfLxxOCONbw0_rl7jY/edit#heading=h.yf95g3tikk1j

Copy link
Copy Markdown
Contributor

@AlexanderS AlexanderS left a comment

Choose a reason for hiding this comment

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

I think we need to move the documentation from the google doc into the rest contract. Currently the whole submissionforms json response is undocumented in most parts, so I do not think, that this should be part of this pull request and can be a follow up one. (But maybe we should create a ticket for it.)

Comment thread submissionforms.md Outdated
"fields": [
{
"input": {
"relationship": true
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we really need this "relationship": true? Couldn't it be implicit, if there are some elements inside selectableRelationship?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It can indeed be implicit, but it may also make it easier for Angular to verify whether it should verify the selectableRelationship section. Either would be fine for me

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would prefer instead to have a:
"type": "relationship"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The "relationship": true has been removed now as discussed in the meeting

@benbosman
Copy link
Copy Markdown
Member Author

@AlexanderS I don't know what the guidelines are for the level of documentation in the rest contracts.
There was no documentation on this file, so only documenting the additions for relationships may not make much sense.
So far, I don't think there's been any documentation on json responses in any of the rest contracts

Comment thread submissionforms.md Outdated
paulo-graca added a commit to paulo-graca/Rest7Contract that referenced this pull request Jul 16, 2019
…lookups in the submission forms...

According to the document - https://docs.google.com/document/d/1X0XsppZYOtPtbmq7yXwmu7FbMAfLxxOCONbw0_rl7jY/edit#heading=h.5bu9shx0j942

You will have what it's called Closed or Open Relations. A closed relation is defined when you have fields in a submission form that relate to Entities, but those fields are "stuck" to Entities, those fields will not have values mixing Entities and text values. For Open Relations you will need a special solution if you want to mix text valued and Entities. @benbosman and Atmire proposal for that, include a flag parameter ("relationship": true) to enable relationships in existing field types. 

We have a different vision for enabling relationships at the form submission level, which should include the creation of two new and different components to accomplish this open and closed relations concept. The "relationship" type should be used for closed relations and "relationship_open" when you need to mix with text values.

The main reasons for this approach has to do with simplicity (don't introduce more complexity in the existing form field types), separation of responsibilities (you will have specific field types to handle the relationships, if one doesn't want to use Entities in DSpace, he can use the already existing ones) and historically DSpace introduced different field types (onebox, twobox, names - all text fields) when the need to add more fields occurred.
 
But I also see the advantages in the solution from Atmire, like with a single flag you will open the fields configuration for Relationships, but at the same time that property shouldn't apply for all field types, for instance textareas, or dates, or even dropdown (controlled values). In theory, you will not have open relations for this kind of fields, there are only some use cases when this might happen and the only one that I can recall it's the author's name.
Copy link
Copy Markdown
Contributor

@AlexanderS AlexanderS left a comment

Choose a reason for hiding this comment

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

👍 Looks good now.

Copy link
Copy Markdown
Contributor

@paulo-graca paulo-graca left a comment

Choose a reason for hiding this comment

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

The group decided to go ahead with this solution.
In the mean time, I've created a JIRA issue to address an alternative approach - https://jira.duraspace.org/browse/DS-4302 - if we plan to revisit this.

Copy link
Copy Markdown
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

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

👍 Looks fine to me. Merging as this is at +3

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.

4 participants