#3215: Use report authors (plural)

Remove `reports.authorUuid` column and migrate authors to `reportPeople.isAuthor`. Replace GraphQL query `report.author` by `report.authors`. Update server-side tests. Note: FIXME's have been added for e-mail templates and `PersonDao::mergePeople`.
NCI-Agency · Oct 12, 2020 · a21b2c7 · a21b2c7
1 parent 7116fd8
commit a21b2c7
Show file tree

Hide file tree

Showing 37 changed files with 311 additions and 239 deletions.
diff --git a/insertBaseData-mssql.sql b/insertBaseData-mssql.sql
diff --git a/mssql2pg.pl b/mssql2pg.pl
@@ -22,6 +22,8 @@
 # Quote mixed-case column and table names
 s/(?<![":])\b([a-z]\w+[A-Z]\w+)/"$1"/g;
 # Turn a couple very specific 1/0 booleans into true/false booleans
+# This one is for "isAuthor" in "reportPeople"
+s/(?<=\Q:reportuuid, \E[10]\Q, \E)([10])/$1 ? 'TRUE' : 'FALSE'/ie;
 # This one is for "isPrimary" in "reportPeople"
 s/(?<=\Q:reportuuid, \E)([10])/$1 ? 'TRUE' : 'FALSE'/ie;
 # This one is for "deleted" in "positionRelationships"
@@ -31,8 +33,8 @@
 }
 # This one is for populating report authorization groups
 s/(?<=rp.\"isPrimary\")\s+=\s+([10])/"= " . ($1 ? 'TRUE' : 'FALSE')/e;
-# This one for the old reports' authors' isAuthor flag update
-s/SET "isAuthor" = 1/ SET "isAuthor" = TRUE/;
+# This one is for populating report approval steps
+s/(?<=\"reportPeople\".\"isAuthor\")\s+=\s+([10])/"= " . ($1 ? 'TRUE' : 'FALSE')/e;
 # standard date-time math would be so nice...
 s/DATEADD\s*\(([^,]*),\s*(-?\d+),\s*CURRENT_TIMESTAMP\)/CURRENT_TIMESTAMP + INTERVAL '$2 $1'/g;
 s/cast\((\S+) as datetime2\((\d+)\)\)/"date_trunc(" . ($2 eq '3' ? "'milliseconds'" : "'second'") . ", $1)"/ie;

diff --git a/src/main/java/mil/dds/anet/beans/Report.java b/src/main/java/mil/dds/anet/beans/Report.java
@@ -95,8 +95,6 @@ public enum ReportCancelledReason {
   @GraphQLInputField
   String reportText;
   // annotated below
-  private ForeignObjectHolder<Person> author = new ForeignObjectHolder<>();
-  // annotated below
   private ForeignObjectHolder<Organization> advisorOrg = new ForeignObjectHolder<>();
   // annotated below
   private ForeignObjectHolder<Organization> principalOrg = new ForeignObjectHolder<>();
@@ -105,6 +103,8 @@ public enum ReportCancelledReason {
   // annotated below
   private ForeignObjectHolder<ReportPerson> primaryPrincipal = new ForeignObjectHolder<>();
   // annotated below
+  private List<ReportPerson> authors;
+  // annotated below
   List<Comment> comments;
   // annotated below
   private List<Tag> tags;
@@ -329,6 +329,26 @@ public ReportPerson getPrimaryPrincipal() {
     return primaryPrincipal.getForeignObject();
   }
 
+  @GraphQLQuery(name = "authors")
+  public CompletableFuture<List<ReportPerson>> loadAuthors(
+      @GraphQLRootContext Map<String, Object> context) {
+    if (authors != null) {
+      return CompletableFuture.completedFuture(authors);
+    }
+    return loadAttendees(context) // Force the load of attendees
+        .thenApply(l -> {
+          final List<ReportPerson> o =
+              l.stream().filter(p -> p.isAuthor()).collect(Collectors.toList());
+          authors = o;
+          return o;
+        });
+  }
+
+  @JsonIgnore
+  public List<ReportPerson> getAuthors() {
+    return authors;
+  }
+
   @GraphQLQuery(name = "tasks")
   public CompletableFuture<List<Task>> loadTasks(@GraphQLRootContext Map<String, Object> context) {
     if (tasks != null) {
@@ -374,37 +394,6 @@ public void setNextSteps(String nextSteps) {
     this.nextSteps = Utils.trimStringReturnNull(nextSteps);
   }
 
-  @GraphQLQuery(name = "author")
-  public CompletableFuture<Person> loadAuthor(@GraphQLRootContext Map<String, Object> context) {
-    if (author.hasForeignObject()) {
-      return CompletableFuture.completedFuture(author.getForeignObject());
-    }
-    return new UuidFetcher<Person>().load(context, IdDataLoaderKey.PEOPLE, author.getForeignUuid())
-        .thenApply(o -> {
-          author.setForeignObject(o);
-          return o;
-        });
-  }
-
-  @JsonIgnore
-  public void setAuthorUuid(String authorUuid) {
-    this.author = new ForeignObjectHolder<>(authorUuid);
-  }
-
-  @JsonIgnore
-  public String getAuthorUuid() {
-    return author.getForeignUuid();
-  }
-
-  @GraphQLInputField(name = "author")
-  public void setAuthor(Person author) {
-    this.author = new ForeignObjectHolder<>(author);
-  }
-
-  public Person getAuthor() {
-    return author.getForeignObject();
-  }
-
   @GraphQLQuery(name = "advisorOrg")
   public CompletableFuture<Organization> loadAdvisorOrg(
       @GraphQLRootContext Map<String, Object> context) {
@@ -819,9 +808,8 @@ public boolean equals(Object o) {
         && Objects.equals(r.getAtmosphereDetails(), atmosphereDetails)
         && Objects.equals(r.getAttendees(), attendees) && Objects.equals(r.getTasks(), tasks)
         && Objects.equals(r.getReportText(), reportText)
-        && Objects.equals(r.getNextSteps(), nextSteps)
-        && Objects.equals(r.getAuthorUuid(), getAuthorUuid())
-        && Objects.equals(r.getComments(), comments) && Objects.equals(r.getTags(), tags)
+        && Objects.equals(r.getNextSteps(), nextSteps) && Objects.equals(r.getComments(), comments)
+        && Objects.equals(r.getTags(), tags)
         && Objects.equals(r.getReportSensitiveInformation(), reportSensitiveInformation)
         && Objects.equals(r.getAuthorizationGroups(), authorizationGroups)
         && Objects.equals(r.getCustomFields(), customFields);
@@ -830,7 +818,7 @@ public boolean equals(Object o) {
   @Override
   public int hashCode() {
     return Objects.hash(uuid, state, approvalStep, createdAt, updatedAt, location, intent, exsum,
-        attendees, tasks, reportText, nextSteps, author, comments, atmosphere, atmosphereDetails,
+        attendees, tasks, reportText, nextSteps, comments, atmosphere, atmosphereDetails,
         engagementDate, duration, tags, reportSensitiveInformation, authorizationGroups);
   }
 

diff --git a/src/main/java/mil/dds/anet/database/PersonDao.java b/src/main/java/mil/dds/anet/database/PersonDao.java
@@ -376,11 +376,12 @@ public int mergePeople(Person winner, Person loser) {
         "UPDATE \"reportActions\" SET \"personUuid\" = :winnerUuid WHERE \"personUuid\" = :loserUuid")
         .bind("winnerUuid", winner.getUuid()).bind("loserUuid", loser.getUuid()).execute();
 
-    // report author update
-    getDbHandle()
-        .createUpdate(
-            "UPDATE reports SET \"authorUuid\" = :winnerUuid WHERE \"authorUuid\" = :loserUuid")
-        .bind("winnerUuid", winner.getUuid()).bind("loserUuid", loser.getUuid()).execute();
+    // FIXME: report author update
+    /*
+     * getDbHandle() .createUpdate(
+     * "UPDATE reports SET \"authorUuid\" = :winnerUuid WHERE \"authorUuid\" = :loserUuid")
+     * .bind("winnerUuid", winner.getUuid()).bind("loserUuid", loser.getUuid()).execute();
+     */
 
     // comment author update
     getDbHandle()

diff --git a/src/main/java/mil/dds/anet/database/ReportDao.java b/src/main/java/mil/dds/anet/database/ReportDao.java
@@ -68,8 +68,8 @@ public class ReportDao extends AnetBaseDao<Report, ReportSearchQuery> {
   public static final String[] minimalFields = {"uuid", "approvalStepUuid",
       "advisorOrganizationUuid", "createdAt", "updatedAt", "engagementDate", "releasedAt", "state"};
   public static final String[] additionalFields = {"duration", "intent", "exsum", "locationUuid",
-      "principalOrganizationUuid", "authorUuid", "atmosphere", "cancelledReason",
-      "atmosphereDetails", "text", "keyOutcomes", "nextSteps", "customFields"};
+      "principalOrganizationUuid", "atmosphere", "cancelledReason", "atmosphereDetails", "text",
+      "keyOutcomes", "nextSteps", "customFields"};
   public static final String[] allFields =
       ObjectArrays.concat(minimalFields, additionalFields, String.class);
   public static final String TABLE_NAME = "reports";
@@ -112,12 +112,12 @@ public Report insertInternal(Report r, Person user) {
     // MSSQL requires explicit CAST when a datetime2 might be NULL.
     StringBuilder sql = new StringBuilder("/* insertReport */ INSERT INTO reports "
         + "(uuid, state, \"createdAt\", \"updatedAt\", \"locationUuid\", intent, exsum, "
-        + "text, \"keyOutcomes\", \"nextSteps\", \"authorUuid\", "
+        + "text, \"keyOutcomes\", \"nextSteps\", "
         + "\"engagementDate\", \"releasedAt\", duration, atmosphere, \"cancelledReason\", "
         + "\"atmosphereDetails\", \"advisorOrganizationUuid\", "
         + "\"principalOrganizationUuid\", \"customFields\") VALUES "
         + "(:uuid, :state, :createdAt, :updatedAt, :locationUuid, :intent, "
-        + ":exsum, :reportText, :keyOutcomes, :nextSteps, :authorUuid, ");
+        + ":exsum, :reportText, :keyOutcomes, :nextSteps, ");
     if (DaoUtils.isMsSql()) {
       sql.append("CAST(:engagementDate AS datetime2), CAST(:releasedAt AS datetime2), ");
     } else {
@@ -501,15 +501,17 @@ public List<Map<String, Object>> getAdvisorReportInsights(Instant start, Instant
     sql.append("%3$s");
     sql.append("%4$s");
     sql.append(" " + String.format(getWeekFormat(), "reports.\"createdAt\"") + " AS week,");
-    sql.append("COUNT(reports.\"authorUuid\") AS \"nrReportsSubmitted\"");
+    sql.append("COUNT(\"reportPeople\".\"personUuid\") AS \"nrReportsSubmitted\"");
 
     sql.append(" FROM ");
     sql.append("positions,");
     sql.append("reports,");
+    sql.append("\"reportPeople\",");
     sql.append("%5$s");
     sql.append("organizations");
 
-    sql.append(" WHERE positions.\"currentPersonUuid\" = reports.\"authorUuid\"");
+    sql.append(" WHERE positions.\"currentPersonUuid\" = \"reportPeople\".\"personUuid\"");
+    sql.append(" AND \"reportPeople\".\"reportUuid\" = reports.uuid");
     sql.append(" %6$s");
     sql.append(" AND reports.\"advisorOrganizationUuid\" = organizations.uuid");
     sql.append(" AND positions.type = :positionAdvisor");
@@ -784,12 +786,14 @@ public CompletableFuture<List<Report>> getReportsBySearch(Map<String, Object> co
   public void sendApprovalNeededEmail(Report r, ApprovalStep approvalStep) {
     final AnetObjectEngine engine = AnetObjectEngine.getInstance();
     final List<Position> approvers = approvalStep.loadApprovers(engine.getContext()).join();
+    final List<ReportPerson> authors = r.loadAuthors(engine.getContext()).join();
     final AnetEmail approverEmail = new AnetEmail();
     final ApprovalNeededEmail action = new ApprovalNeededEmail();
     action.setReport(r);
     approverEmail.setAction(action);
     approverEmail.setToAddresses(approvers.stream()
-        .filter(a -> (a.getPersonUuid() != null) && !a.getPersonUuid().equals(r.getAuthorUuid()))
+        .filter(a -> (a.getPersonUuid() != null)
+            && authors.stream().filter(p -> a.getPersonUuid().equals(p.getUuid())).count() == 0)
         .map(a -> a.loadPerson(engine.getContext()).join().getEmailAddress())
         .collect(Collectors.toList()));
     AnetEmailWorker.sendEmailAsync(approverEmail);
@@ -800,8 +804,8 @@ public void sendReportPublishedEmail(Report r) {
     final ReportPublishedEmail action = new ReportPublishedEmail();
     action.setReport(r);
     email.setAction(action);
-    email.addToAddress(
-        r.loadAuthor(AnetObjectEngine.getInstance().getContext()).join().getEmailAddress());
+    email.setToAddresses(r.loadAuthors(AnetObjectEngine.getInstance().getContext()).join().stream()
+        .map(rp -> rp.getEmailAddress()).collect(Collectors.toList()));
     AnetEmailWorker.sendEmailAsync(email);
   }
 
@@ -828,7 +832,9 @@ public int approve(Report r, Person user, ApprovalStep step) {
         r.setState(ReportState.APPROVED);
       }
     }
-    final int numRows = update(r, r.getAuthor());
+    final Optional<ReportPerson> firstAuthor =
+        r.loadAuthors(AnetObjectEngine.getInstance().getContext()).join().stream().findFirst();
+    final int numRows = update(r, firstAuthor.orElse(null));
     if (numRows != 0 && nextStep != null) {
       sendApprovalNeededEmail(r, nextStep);
     }
@@ -875,7 +881,9 @@ public int publish(Report r, Person user) {
     // Move the report to PUBLISHED state
     r.setState(ReportState.PUBLISHED);
     r.setReleasedAt(Instant.now());
-    final int numRows = update(r, r.getAuthor());
+    final Optional<ReportPerson> firstAuthor =
+        r.loadAuthors(AnetObjectEngine.getInstance().getContext()).join().stream().findFirst();
+    final int numRows = update(r, firstAuthor.orElse(null));
     if (numRows != 0) {
       sendReportPublishedEmail(r);
     }
@@ -893,8 +901,7 @@ public List<Report> getFutureToPastReports(Instant end) {
     StringBuilder sql = new StringBuilder();
 
     sql.append("/* getFutureToPastReports */");
-    sql.append(
-        " SELECT reports.uuid AS reports_uuid, reports.\"authorUuid\" AS \"reports_authorUuid\"");
+    sql.append(" SELECT reports.uuid AS reports_uuid");
     sql.append(" FROM reports");
     // We are not interested in draft reports, as they will remain draft.
     // We are not interested in cancelled reports, as they will remain cancelled.

diff --git a/src/main/java/mil/dds/anet/database/ReportSensitiveInformationDao.java b/src/main/java/mil/dds/anet/database/ReportSensitiveInformationDao.java
@@ -3,7 +3,6 @@
 import java.time.Instant;
 import java.util.List;
 import java.util.Map;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
 import mil.dds.anet.AnetObjectEngine;
@@ -67,7 +66,8 @@ public ReportSensitiveInformation insertInternal(ReportSensitiveInformation rsi)
   @InTransaction
   public ReportSensitiveInformation insert(ReportSensitiveInformation rsi, Person user,
       Report report) {
-    if (rsi == null || !isAuthorized(user, report) || Utils.isEmptyHtml(rsi.getText())) {
+    if (rsi == null || !isAuthorized(AnetObjectEngine.getInstance().getContext(), user, report)
+        || Utils.isEmptyHtml(rsi.getText())) {
       return null;
     }
     DaoUtils.setInsertFields(rsi);
@@ -89,7 +89,7 @@ public int updateInternal(ReportSensitiveInformation rsi) {
 
   @InTransaction
   public int update(ReportSensitiveInformation rsi, Person user, Report report) {
-    if (rsi == null || !isAuthorized(user, report)) {
+    if (rsi == null || !isAuthorized(AnetObjectEngine.getInstance().getContext(), user, report)) {
       return 0;
     }
     final int numRows;
@@ -116,7 +116,7 @@ public Object insertOrUpdate(ReportSensitiveInformation rsi, Person user, Report
   @InTransaction
   public CompletableFuture<ReportSensitiveInformation> getForReport(Map<String, Object> context,
       Report report, Person user) {
-    if (!isAuthorized(user, report)) {
+    if (!isAuthorized(context, user, report)) {
       return CompletableFuture.completedFuture(null);
     }
     return new ForeignKeyFetcher<ReportSensitiveInformation>()
@@ -134,21 +134,21 @@ public CompletableFuture<ReportSensitiveInformation> getForReport(Map<String, Ob
 
   /**
    * A user is allowed to access a report's sensitive information if either of the following holds
-   * true: • the user is the author of the report; • the user is in an authorization group for the
+   * true: • the user is an author of the report; • the user is in an authorization group for the
    * report.
    *
    * @param user the user executing the request
    * @param report the report
    * @return true if the user is allowed to access the report's sensitive information
    */
-  private boolean isAuthorized(Person user, Report report) {
+  private boolean isAuthorized(Map<String, Object> context, Person user, Report report) {
     final String userUuid = DaoUtils.getUuid(user);
     final String reportUuid = DaoUtils.getUuid(report);
     if (userUuid == null || reportUuid == null) {
       // No user or no report
       return false;
     }
-    if (Objects.equals(userUuid, report.getAuthorUuid())) {
+    if (DaoUtils.isReportAuthor(user, report)) {
       // Author is always authorized
       return true;
     }

diff --git a/src/main/java/mil/dds/anet/database/mappers/ReportMapper.java b/src/main/java/mil/dds/anet/database/mappers/ReportMapper.java
@@ -34,7 +34,6 @@ public Report map(ResultSet rs, StatementContext ctx) throws SQLException {
     r.setKeyOutcomes(MapperUtils.getOptionalString(rs, "reports_keyOutcomes"));
     r.setNextSteps(MapperUtils.getOptionalString(rs, "reports_nextSteps"));
 
-    r.setAuthorUuid(MapperUtils.getOptionalString(rs, "reports_authorUuid"));
     r.setAdvisorOrgUuid(MapperUtils.getOptionalString(rs, "reports_advisorOrganizationUuid"));
     r.setPrincipalOrgUuid(MapperUtils.getOptionalString(rs, "reports_principalOrganizationUuid"));