Skip to content

Add support for removing fields by index or by item/type#75

Closed
nkm8 wants to merge 2 commits intonHapiNET:masterfrom
nkm8:scratch
Closed

Add support for removing fields by index or by item/type#75
nkm8 wants to merge 2 commits intonHapiNET:masterfrom
nkm8:scratch

Conversation

@nkm8
Copy link

@nkm8 nkm8 commented Mar 17, 2017

I am surprised no one has required this functionality before, but I need the ability to remove specific instances of repeatable fields. This is not an ideal solution because I don't have the means (documentation) to re-generate the Model classes from a database properly. However, this solution is flexible enough to cover my use case.

This functionality already exists in hapi: http://hl7api.sourceforge.net/v26/apidocs/src-html/ca/uhn/hl7v2/model/v26/segment/PV1.html#line.388

FindField was added as a helper method because I also have a pull request dib0/NHapiTools#5 to update nHapiTools to generate extension methods for removing repeatable fields.

I am open to alternative approaches or any suggestions you may have.

@dreaminhex
Copy link
Collaborator

HAPI link is a dead link. As I have no context for this change, I'll close. If still needed or reopened, please provide updated link for comparison.

@dreaminhex dreaminhex closed this Oct 22, 2020
@milkshakeuk
Copy link
Member

milkshakeuk commented Oct 23, 2020

@Usualdosage the closest link I found which exists is:
https://hapifhir.github.io/hapi-hl7v2/v26/apidocs/ca/uhn/hl7v2/model/v26/segment/PV1.html
I think he is referring to the functions that remove repetitions.

@nkm8 can you find in the new documentation what you were referring to?

@milkshakeuk
Copy link
Member

@milkshakeuk
Copy link
Member

to do this we would need to add functionality to AbstractSegment.cs

@milkshakeuk milkshakeuk reopened this Oct 28, 2020
@milkshakeuk
Copy link
Member

@nkm8 its hard to tell what your changes are because it looks like the methods have been moved around, could you update the PR to only show the changes required to add the functionality.

@nkm8
Copy link
Author

nkm8 commented Nov 2, 2020

@milkshakeuk There weren't any merge conflicts, but I pulled from master and updated the PR. If you ignore the whitespace changes, the PR is straight-forward: https://github.com/nHapiNET/nHapi/pull/75/files?diff=split&w=1

Copy link
Member

@milkshakeuk milkshakeuk left a comment

Choose a reason for hiding this comment

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

based on the discussion, this seems fine, I may add some unit tests at some point anyway.

@milkshakeuk milkshakeuk added this to the v3.0.0.0 milestone Nov 3, 2020
milkshakeuk added a commit that referenced this pull request Nov 3, 2020
@milkshakeuk
Copy link
Member

Manually merged to avoid merge bubble of nhapi/master into nkm8/scratch and better preserve the git history as if nkm8/scratch had been rebased first before the PR merge..

@milkshakeuk milkshakeuk closed this Nov 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants