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

Pass HL7Globals to MSH in ACK #49

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

pluk77
Copy link
Contributor

@pluk77 pluk77 commented May 26, 2021

Ensure the HL7 globals are passed to the MSH when creating an ACK message. Part of #22

Ensure the HL7 globlas are passed to the MSH when creating an ACK message
@senaranya
Copy link
Owner

Can you please add tests?

Passing globals together with a message is pointless
@pluk77
Copy link
Contributor Author

pluk77 commented Jun 1, 2021

I perhaps can, but can you explain the following existing test?
Although you pass the HL7 version to the constructor of the ACK message, you are not expecting it to be returned as I do not see the HL7 version in the assertSame() string?

Isn't the whole reason for adding the globals to the constructor so the values are used when creating the message?

   public function globals_can_be_passed_to_constructor(): void
    {
        $msg = new Message("MSH|^~\\&|1|\rPV1|1|O|^AAAA1^^^BB|");
        $ack = new ACK($msg, null, ['HL7_VERSION' => '2.5']);
        self::assertSame("MSH|^~\&|1||||||ACK|\nMSA|AA|\n", $ack->toString(true));
    }

@senaranya
Copy link
Owner

You're absolutely right - the test doesn't check for globals properly. I've replaced it with a proper test. Please rebase your branch on to master to get it.

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.

None yet

2 participants