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

Updates migrations with using field_unique_id as default for parse_entity_lookups #204

Merged
merged 17 commits into from
Aug 16, 2021

Conversation

bseeger
Copy link

@bseeger bseeger commented Aug 11, 2021

Resovles: jhu-idc/iDC-general#388

This update migrations to have the parse_entity_lookup and entity_lookup processes use field_unique_id instead of title or name as the default for value_key.

Also update test migrations to use the field_unique_id field information in entity references.

Adds field_unique_id to search index as well as to the exported information.

To test this -- run the test suite or create an ingest using field_unique_id to reference an Entity Reference.

To be clear, all the places where user manage taxonomy terms might be applied use the parse_entity_lookup. All the places that use a system managed taxonomy (Islandora Media Use, Islandora Display and Islandora Models) still use entity_lookup as they don't have a field_unique_id.

field_unique_id, instead of name/title.
Still a WIP and will need changes.
@bseeger bseeger force-pushed the migration_unique_id_updates branch from c5f1675 to 5046172 Compare August 11, 2021 13:59
@github-actions
Copy link

This PR has no dependency differences with the base branch

@github-actions
Copy link

This PR has no dependency differences with the base branch

@github-actions
Copy link

This PR has no dependency differences with the base branch

@github-actions
Copy link

This PR has no dependency differences with the base branch

@github-actions
Copy link

This PR has no dependency differences with the base branch

@github-actions
Copy link

This PR has no dependency differences with the base branch

@github-actions
Copy link

This PR has no dependency differences with the base branch

and to not fail there's an unexpected message (like a message about
drupal needing updating)
@github-actions
Copy link

This PR has no dependency differences with the base branch

@github-actions
Copy link

This PR has no dependency differences with the base branch

(it had a typo, but I'm not sure it's needed in the first place)
@github-actions
Copy link

This PR has no dependency differences with the base branch

@github-actions
Copy link

This PR has no dependency differences with the base branch

@github-actions
Copy link

This PR has no dependency differences with the base branch

@github-actions
Copy link

This PR has no dependency differences with the base branch

@github-actions
Copy link

This PR has no dependency differences with the base branch

@github-actions
Copy link

This PR has no dependency differences with the base branch

@github-actions
Copy link

This PR has no dependency differences with the base branch

@bseeger bseeger changed the title WIP - updates migrations with using field_unique_id as default for parse_entity_lookups Updates migrations with using field_unique_id as default for parse_entity_lookups Aug 16, 2021
@birkland birkland self-requested a review August 16, 2021 18:48
Copy link

@birkland birkland left a comment

Choose a reason for hiding this comment

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

This is great! One optional suggestion to document new argument in util.js and rename it to accessTerm, pointed out in four separate comments. Feel free to just merge if you make that change, and the build passes

@@ -166,7 +180,7 @@ export const download = async (uri) => {
* @param {string} name Name of the repository object for whom to upload an image to
* @param {string} file path of the file to upload, on disk.
*/
export const uploadImageInUI = async (t, name, file) => {
export const uploadImageInUI = async (t, name, file, access) => {

Choose a reason for hiding this comment

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

rename to accessTerm, add something like @param {string} accessTerm textual name of access term to use for the uploaded file

@@ -185,7 +200,7 @@ export const uploadImageInUI = async (t, name, file) => {
* @param {string} name Name of the repository object for whom to upload a file to
* @param {string} file path of the file to upload, on disk.
*/
export const uploadFileInUI = async (t, name, file) => {
export const uploadFileInUI = async (t, name, file, access) => {

Choose a reason for hiding this comment

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

rename to accessTerm, add something like @param {string} accessTerm textual name of access term to use for the uploaded file

@@ -166,7 +182,7 @@ export const download = async (uri) => {
* @param {string} name Name of the repository object for whom to upload an image to
* @param {string} file path of the file to upload, on disk.
*/
export const uploadImageInUI = async (t, name, file) => {
export const uploadImageInUI = async (t, name, file, access) => {

Choose a reason for hiding this comment

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

rename to accessTerm, add something like @param {string} accessTerm textual name of access term to use for the uploaded file

@@ -185,7 +202,7 @@ export const uploadImageInUI = async (t, name, file) => {
* @param {string} name Name of the repository object for whom to upload a file to
* @param {string} file path of the file to upload, on disk.
*/
export const uploadFileInUI = async (t, name, file) => {
export const uploadFileInUI = async (t, name, file, access) => {

Choose a reason for hiding this comment

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

rename to accessTerm, add something like @param {string} accessTerm textual name of access term to use for the uploaded file

@jabrah jabrah mentioned this pull request Aug 16, 2021
adds a description for the parameter
@github-actions
Copy link

This PR has no dependency differences with the base branch

@bseeger bseeger merged commit 418083a into development Aug 16, 2021
@birkland birkland deleted the migration_unique_id_updates branch August 16, 2021 20:55
jaredgalanis pushed a commit that referenced this pull request Feb 15, 2022
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.

2 participants