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

Issue #2365 - Show resource ids in conditional delete operation outcome #2367

Merged
merged 1 commit into from
May 14, 2021

Conversation

tbieste
Copy link
Contributor

@tbieste tbieste commented May 14, 2021

Issue #2365 - Show resource ids in conditional delete operation outcome

Signed-off-by: Troy Biesterfeld tbieste@us.ibm.com

Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
@tbieste tbieste requested a review from lmsurpre May 14, 2021 15:19
@tbieste tbieste marked this pull request as ready for review May 14, 2021 15:19
@tbieste tbieste self-assigned this May 14, 2021
Copy link
Contributor

@michaelwschroeder michaelwschroeder left a comment

Choose a reason for hiding this comment

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

LGTM

@tbieste tbieste merged commit daf2f95 into main May 14, 2021
@tbieste tbieste deleted the tbieste-issue-2365 branch May 14, 2021 17:29
@@ -2563,6 +2563,7 @@ Bundle createSearchBundle(List<Resource> resources, FHIRSearchContext searchCont
Entry.Builder entryBuilder = Entry.builder();
if (resource != null) {
if (resource.getId() != null) {
entryBuilder.id(resource.getId());
Copy link
Member

Choose a reason for hiding this comment

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

why are we using the Entry.id for this?

Copy link
Member

Choose a reason for hiding this comment

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

I guess its because there is no resource in the response entries, and so we can't use the normal Entry.resource.id. Still, I think we might want a comment for this because using Entry.id for the id of the deleted resource is a bit unique.

Copy link
Member

Choose a reason for hiding this comment

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

side-note: you definitely didn't introduce this idea of using the entry id for this. that comes from
FHIRRestHelper.doDelete near line 722:

responseBundle.getEntry().stream().map(Bundle.Entry::getId).collect(Collectors.joining(","))

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.

3 participants