Skip to content

issue 964 properly impl getEntityTypeByLabel#965

Merged
tdonohue merged 2 commits intoDSpace:mainfrom
atmire:Issue-964-getEntityTypeByLabel-impl
Dec 11, 2020
Merged

issue 964 properly impl getEntityTypeByLabel#965
tdonohue merged 2 commits intoDSpace:mainfrom
atmire:Issue-964-getEntityTypeByLabel-impl

Conversation

@AndrewZachWood
Copy link
Contributor

@AndrewZachWood AndrewZachWood commented Dec 7, 2020 •
edited by artlowel
Loading

Fixes #964

@AndrewZachWood AndrewZachWood mentioned this pull request Dec 7, 2020
Closed
@artlowel artlowel added the 1 APPROVAL pull request only requires a single approval to merge label Dec 8, 2020
@AndrewZachWood AndrewZachWood force-pushed the Issue-964-getEntityTypeByLabel-impl branch from dbbd298 to 10e09b6 Compare December 10, 2020 16:08
artlowel
artlowel requested changes Dec 10, 2020
View reviewed changes
Copy link
Member

@artlowel artlowel left a comment

Choose a reason for hiding this comment

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

Thanks @AndrewZWood!

Theres an issue with the implementation. See my inline comment.

@artlowel artlowel mentioned this pull request Dec 11, 2020
Closed
artlowel
artlowel approved these changes Dec 11, 2020
View reviewed changes
Copy link
Member

@artlowel artlowel left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks @AndrewZWood, I've verified that your code works now.

However it has brought a bug on the rest side to light. The entity types retrieved in this way can't be parsed, because they're lacking a _links section. I created an issue for it.

So this PR should not be merged until that issue is fixed on the backend

@AndrewZachWood
Copy link
Contributor Author

AndrewZachWood commented Dec 11, 2020

@artlowel Already have an issue and PR for what you're describing: DSpace/DSpace#3078

tdonohue
tdonohue approved these changes Dec 11, 2020
View reviewed changes
Copy link
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.

👍 I tested this alongside DSpace/DSpace#3079 (corresponding REST PR) and they work great! Thanks @AndrewZWood !

I'll merge both together & ensure the REST PR is deployed to the demo site.

@tdonohue tdonohue merged commit 17ca2c4 into DSpace:main Dec 11, 2020
@tdonohue tdonohue added this to the 7.0beta5 milestone Jan 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tdonohue tdonohue tdonohue approved these changes

@artlowel artlowel artlowel approved these changes

Assignees

No one assigned

Labels

1 APPROVAL pull request only requires a single approval to merge

Projects

None yet

Milestone

7.0beta5

Development

Successfully merging this pull request may close these issues.

EntityTypeService getEntityTypeByLabel is mocked: needs proper implementation

3 participants

@AndrewZachWood @tdonohue @artlowel