Issue Number | 749 |
---|---|
Summary | [Hotel & Reimbursement Requests] Send to group email |
Created | 2023-03-13 12:25:39 |
Issue Type | Improvement |
Submitted By | Shields, Victoria (NIH/NCI) [E] |
Assigned To | Kline, Bob (NIH/NCI) [C] |
Status | Closed |
Resolved | 2023-04-10 07:02:58 |
Resolution | Fixed |
Path | /home/bkline/backups/jira/oceebms/issue.340777 |
Please add these emails as recipients for Hotel Requests and Reimbursement Requests:
On production? Or some other tier(s)?
On production. Thanks Bob!
New addresses added.
Sorry, could you please add one more email as a recipient of both
forms?
NCITravel@jbsinternational.com
Thanks.
Hmm, teensy problem. We've gone 10 years in the CDR with just two addresses, so it's unexpected that we'd exceed the limit of 128 characters. This is all I can fit right now
.ziscovici@nih.gov,dominic.cooper@nih.gov,robin.juthe@nih.gov,vshields@mail.nih.gov,nciocplpdq@mail.nih.gov,NCITravel@jbs charles
We can add a ticket to Fiordland to see if we can enlarge that limit, but for right now we'll have to decide which addresses are really needed.
Good news for now - we can remove Charles and Dominic since they no
longer work here. That should give us enough room for the full JBS
email. Thanks!
charles.ziscovici@nih.gov,dominic.cooper@nih.gov,
Good news for now - we can remove Charles and Dominic since they no longer work here.
Excellent! We're all set.
(I'm moving away from using "Reply" in JIRA. My current working theory is that struggles to keep track of the reply threading is contributing to—if not causing—the "disappearing comments" bugs.)
Well, that's bizarre. I hit "Add Comment" instead of "Reply" for that last comment, but JIRA still made it a reply to some other random comment. And it won't let me delete it. 🙁
Anyway, the travel manager email addresses are all set.
Unfortunately, Terry at JBS still isn't receiving these emails from the EBMS. Just wanted to confirm that the change was made on PROD. If so, could you please also make the change on a lower tier (whichever one you think is most appropriate) so we can do some testing? Thanks.
OK. Now DEV and PROD have the same value for the Travel Manager Email Address(es) field:
robin.juthe@nih.gov,vshields@mail.nih.gov,nciocplpdq@mail.nih.gov,NCITravel@jbsinternational.com
Please check what I have for typos (and test on DEV, of course 😉).
I would think, however, that since you're one of the recipients, and (as far as I know) the mail server we're using doesn't do that trick where each recipient's copy looks like that recipient is the ONLY recipient, you should probably be seeing all of the addresses on the TO line, right? So if NCITravel@jbsinternational.com is one of the addresses you see, then i would say the problem is not likely to be in the EBMS software (by which I mean, it can't be, unless my understanding of how email works is a complete shambles, which would be a really bad sign for how well my cognitive facilities are holding up, given all of the email gateways I wrote earlier in my career). Anyway, I look forward to hearing the results of your testing.
First piece of information is a minor bit of good news: the server name (showing the tier) is only added for the non-production servers, so the fact that you're not seeing it for the production messages is not a problem. In fact, that's how you know it's prod: it doesn't show the server address.
The next piece of information is not such good news. There's a bug in the code which is causing the messages to fail, but only on production. The page where I set the recipients is setting the value of the
.email.travel_manager email_travel
variable. But the reimbursement request for and the hotel request form are both looking for the
.email.manager email_travel
variable.
So you'll want to put a ticket in to have me fix that bug. Very unfortunate that we didn't test this path. Sorry about that! 🙁
The bug is fixed on DEV and the temporary tweak to the code is in place to make it behave as if it's the production server (so sending the messages to you, Victoria, and me). And you can still test that if you want, though I think the mystery is already solved.
I've confirmed this is working as expected on DEV. I received the emails.
Moved to Fiordland since it can't be completely tested until the parallel ticket for the bug fix (OCEEBMS-751) is deployed to production.
Hi ~bkline , could you please add my email and Victoria's email to the list of recipients for the hotel request and reimbursement request forms on QA? Once I confirm that's working I may also ask you to add the JBS email just to be completely sure. Thank you!
Adding those addresses won't let you test what you want to test here. On every tier except production the software gets its list of recipients from the "developers" configuration variable instead of the "travel_manager" configuration variable. That's a deliberate decision we made so testing on the lower tiers doesn't spam the folks who get the real notifications from activity on the production server. I could manually change the code so that QA thinks it's the production server for this purpose, but then you wouldn't be testing the code we're going to deploy to production next week (give or take). That's why I wrote earlier that this ticket "can't be completely tested until the parallel ticket for the bug fix (OCEEBMS-751) is deployed to production." Make sense?
Remember, the problem wasn't that NIH wasn't able to deliver email to certain addresses. We're pretty confident that whatever valid email addresses we enter will be able to get email delivered by NIH. The problem was that the code sending out the notification was using a different name for the configuration variable than the name being used by the form which lets me set the variable's value. Here's the code which sends out the notification:
/**
* Notify the travel manager that we have a new hotel request.
*
* @param array $values
* Values from the form.
*/
private function sendEmailNotification(array $values) {
// Make sure we have someone to whom we can send the notification.
$subject = 'Hotel Request';
$host = $this->getRequest()->getHost();
if ($host === 'ebms.nci.nih.gov') {
$to = $this->config('ebms_travel.email')->get('travel_manager');
}else {
$to = $this->config('ebms_travel.email')->get('developers');
$subject .= " ($host)";
}
... }
That line which now has get('travel_manager')
used to
be:
$to = $this->config('ebms_travel.email')->get('manager');
Here's the code which stores the values on the form for managing those email addresses:
/**
* {@inheritdoc}
*/
public function submitForm(array &$form, FormStateInterface $form_state) {
$trigger = $form_state->getTriggeringElement()['#value'];
if ($trigger === 'Save') {
.
...$config = $this->configFactory()->getEditable('ebms_travel.email');
$config->set('travel_manager', $form_state->getValue('travel-manager'));
$config->set('developers', $form_state->getValue('developers'));
$config->save();
$this->messenger()->addMessage('Saved travel configuration.');
}$form_state->setRedirect('ebms_travel.landing_page');
}
The important part is the
$config->set('travel_manager', ...);
which has always
been that way. With this fix the code above (sending out the
notification) now matches that the use of that name.
Hope this helps clarify things.
Got it. Thanks, Bob. I was trying to replicate the testing we did on DEV but now I see that you had manually applied the code from prod in order for us to test. Since that was successful and we can't test this completely until the other bug fix is on prod, I think we can proceed without testing this one on QA.
As Bob has noted, we aren't really able to test this issue, but I'm moving to QA Verified.
~bkline , now that this fix has landed on PROD, I just wanted to confirm that the following emails are set up to receive hotel and reimbursement requests on PROD. Thank you!
robin.juthe@nih.gov,vshields@mail.nih.gov,nciocplpdq@mail.nih.gov,NCITravel@jbsinternational.com
Yes, that's correct. Latest hotel request was April 28, and latest reimbursement request was the 17th, though, so we won't have actual evidence until we get a new travel request.
As of today, no one has submitted a hotel or reimbursement request since the release was deployed. The Pediatric Board has an in-person meeting on May 19, so hopefully some submissions will come in the following week.
Looks as if we have a couple of reimbursement requests. Can you confirm that the contractor got the notifications?
I can confirm that these requests are going to the contractor's travel box. Hooray!
I'd like to see a hotel request come through before I close this ticket. The next in-person meetings is for Peds, and we've asked Crystal to reach out to her group and ask them to start making reservations.
I submitted a hotel request for test board member and it was received by the travel contractor. We feel confident that this is working so I'm closing the ticket. Verified on PROD.
Elapsed: 0:00:00.000667