added alternative content tag and controls on bitstreams#4016
added alternative content tag and controls on bitstreams#4016oscar-escire wants to merge 8 commits intoDSpace:mainfrom
Conversation
tdonohue
moved this from 🙋 Needs Reviewers Assigned
to 👀 Under Review
in DSpace 9.0 Release
atarix83
requested review from
steph-ieffam
and removed request for
atarix83
steph-ieffam
left a comment
steph-ieffam
left a comment
There was a problem hiding this comment.
Hey @oscar-escire I added a PR review directly on the Rest repository: DSpace/DSpace#10431 (review)
|
Hi @oscar-escire, |
|
@oscar-escire : This frontend PR has merge conflicts. Could you find time to update this so that it's in a testable state again? |
|
@tdonohue thanks! I have fixed the merge conflicts. |
|
Hi @oscar-escire, |
tdonohue
left a comment
tdonohue
left a comment
There was a problem hiding this comment.
@oscar-escire : Thanks for this contribution. I gave this a more thorough test today, and overall the basics work. But, similar to @steph-ieffam 's comments on the (obsolete) backend PR, I find the behavior a little odd overall.
First, I want to document what I had to do to get this to work properly (as it's not documented in this PR).
- Installed this PR (obviously)
- Add the required "dc.type" field to the "bitstream-metadata" form in the
submission-forms.xml(on the backend):
<row>
<field>
<dc-schema>dc</dc-schema>
<dc-element>type</dc-element>
<repeatable>false</repeatable>
<label>Type</label>
<input-type>onebox</input-type>
<hint>Enter a type of file</hint>
<required></required>
</field>
</row>
- Then I built & ran this PR in production mode:
npm run build:prod && npm run serve:ssr
The end result is that I see this new "Alternative Content" checkbox in both the Submission Form (when editing the bitstream) and in the Edit Item form (again when editing a bitstream)
However, a bug that I noticed is that if I submit an Item though with files marked as "Alternative Content", the files show up oddly on the Item Page. It appears these "Alternative Content" badges have no style? In the below picture, all three files have an "Alternative Content" badge, but it's white on a white background.
Another small bug is that, in the Submission Form, the action buttons next to Bitstreams are no longer next to each other. Here's how it looks currently in our codebase:
But, here's how it now looks in this PR:
Overall, this feature works at a basic level, and the above bugs are relatively small.
That said, I do have two larger concerns.
- I find the checkboxes for Primary Bitstream & Alternative Content to be odd. The Primary Bitstream was previously a switch, and now it's become a checkbox (but only one file can be checked). The "Alternative Content" is a checkbox, but multiple files can be checked, and it doesn't display in the Submission Form next to each file (like the Primary Bitstream does).
- I don't have a great solution here, but I think the toggle for "Primary Bitstream" should not be changed to a checkbox. I'd rather they both be toggles instead of checkboxes,
- I feel the way this "Alternative Content" is stored in metadata is potentially "fragile". Currently, if you check that checkbox, the Bitstream will have a
dc.typeset toalternative content. This is more fragile than how we store the "primary bitstream" flag because it's possible that sites will already usedc.typeon their Bitstreams for a different purpose.- A better approach would be to either store this value in a database column (like primary bitstream) or create a new
dspace.bitstream.typemetadata field (in our internaldspaceschema) which could be used to store this value. That way, we are not using adc.*field that other sites may already use, and we make it clear this is an internal use metadata field (as all otherdspace.*fields are)
- A better approach would be to either store this value in a database column (like primary bitstream) or create a new
Finally, one other note inline below.
| bitstream$ = observableOf(this.bitstream); | ||
| } | ||
| if (isAlternativeContent) { | ||
| updatedBitstream.setMetadata('dc.type', undefined, ...['alternative content']); |
There was a problem hiding this comment.
I don't like how this value of alternative content has to be hardcoded in many areas of this PR. It'd be better to use a single constant that could be referenced in all areas. Since this is a hardcoded value, it also could be shortened to something like alt-content
Alternatively, this value could be moved to the backend as a constant (but that might require a new endpoint of some sort to tell the backend when a bitstream should be flagged as "alternative content")
|
Hello @tdonohue, thanks for your comments. I generally agree with you, I think the recent merges have affected the PR's views. I'll be working on an update and will share it in the coming days. |
|
@oscar-escire : Just a brief note. This unfortunately missed our feature merger deadline for 9.0. Since it's a new feature, I'll have to unfortunately move it to the next major release (10.0). You're still welcome to work on it, but I don't see a way to get this into 9.0 because it'll miss our 9.0 Testathon (starting next week), which is required for all new features. |
|
@oscar-escire : We are just beginning a more detailed review process for 10.0 for all "new feature" PRs. If you can rebase this on latest |
# Conflicts: # src/app/item-page/simple/field-components/file-section/file-section.component.html # src/app/submission/sections/upload/file/edit/section-upload-file-edit.component.ts
|
@tdonohue: I have updated the PR so that it can be considered for DSpace 10 and have applied the following observations:
Is there anything else we should consider? Perhaps this function could be enabled or disabled with a configuration or something similar. |
3 participants
References
Description
This PR allows to set a tag in the list of bitstreams on item page to indicate if its an alternative content, you can set the tag on bitstreams of archived items or in the upload section edit bitstream of a itemworkspace.
Instructions for Reviewers
Alternative:
List of changes in this PR:
Checklist
mainbranch of code (unless it is a backport or is fixing an issue specific to an older branch).npm run lintnpm run check-circ-deps)package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.