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

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

Closed
wants to merge 2 commits into from

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.

@Usualdosage
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.

@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.

@milkshakeuk milkshakeuk added this to In Progress in NHapi Kanban via automation Oct 30, 2020
@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
NHapi Kanban automation moved this from In Progress to Discarded Nov 3, 2020
@milkshakeuk milkshakeuk moved this from Discarded to Development Done in NHapi Kanban Nov 3, 2020
@milkshakeuk milkshakeuk moved this from Development Done to In Pre-Release in NHapi Kanban Jan 30, 2021
@milkshakeuk milkshakeuk moved this from In Pre-Release to Released in NHapi Kanban Jun 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants