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

[DUOS-2780] System Email - DAR submitted to DAC Chair #2236

Merged
merged 15 commits into from
Mar 7, 2024

Conversation

hams7504
Copy link
Contributor

@hams7504 hams7504 commented Jan 26, 2024

Addresses

https://broadworkbench.atlassian.net/browse/DUOS-2780

Summary

  • Reformatted email template to add extra information
  • Added new Dac DAO method that retrieves all dacs in a given collection

Have you read CONTRIBUTING.md lately? If not, do that first.

  • Label PR with a Jira ticket number and include a link to the ticket
  • Label PR with a security risk modifier [no, low, medium, high]
  • PR describes scope of changes
  • Get a minimum of one thumbs worth of review, preferably two if enough team members are available
  • Get PO sign-off for all non-trivial UI or workflow changes
  • Verify all tests go green
  • Test this change deployed correctly and works on dev environment after deployment

@hams7504 hams7504 changed the title Hm duos 2780 dar submitted to chair WIP: duos 2780 dar submitted to chair Jan 26, 2024
@hams7504 hams7504 force-pushed the hm-duos-2780-dar-submitted-to-chair branch from 2684d39 to 42ca0a8 Compare February 6, 2024 16:11
@hams7504 hams7504 changed the title WIP: duos 2780 dar submitted to chair [DUOS-2780] System Email - DAR submitted to DAC Chair Feb 6, 2024
@hams7504 hams7504 marked this pull request as ready for review February 6, 2024 16:41
@hams7504 hams7504 requested a review from a team as a code owner February 6, 2024 16:41
Copy link
Contributor

@rushtong rushtong left a comment

Choose a reason for hiding this comment

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

Going to tackle this feedback myself.

@@ -128,6 +150,26 @@ public List<MailMessage> fetchEmailMessagesByCreateDate(Date start, Date end, In
public void sendNewDARCollectionMessage(Integer collectionId)
throws IOException, TemplateException {
DarCollection collection = collectionDAO.findDARCollectionByCollectionId(collectionId);
List<User> distinctUsers = getDistinctUsers(collection);
DataAccessRequest dar = dataAccessRequestDAO.findByReferenceId(collection.getDarCode());
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to fix this, the dar code is not the same as the reference id.

Comment on lines 224 to 231
@SqlQuery("""
select dac.*
from dac
inner join dataset d on d.dac_id = dac.dac_id
inner join dar_dataset dd on dd.dataset_id = d.dataset_id
inner join data_access_request dar on dd.reference_id = dar.reference_id
where dar.collection_id = :collectionId
""")
Copy link
Contributor

Choose a reason for hiding this comment

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

We should standardize the casing here

Comment on lines 155 to 157
String researcherName = userDAO.findUserById(dar.getUserId()).getDisplayName();
Collection<Dac> dacsInDAR = dacDAO.findDacsForCollectionId(collectionId);
List<Dataset> datasetsInDAR = datasetDAO.findDatasetsByIdList(dar.datasetIds);
Copy link
Contributor

Choose a reason for hiding this comment

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

We need some safety checking around the bulk calls to ensure we're not passing empty lists.

Copy link
Contributor

@rushtong rushtong left a comment

Choose a reason for hiding this comment

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

Manually tested 👍🏽

Copy link

@Shakespeared Shakespeared left a comment

Choose a reason for hiding this comment

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

seems reasonable, lgtm but didn't manually test

@rushtong rushtong merged commit bfd1137 into develop Mar 7, 2024
11 checks passed
@rushtong rushtong deleted the hm-duos-2780-dar-submitted-to-chair branch March 7, 2024 19:22
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