!feat: enable user defined 'id' for reports so that the get URL can be stable. Breaking change as 'id' will be required to function.

apps/reports/src/components/ListReports.vue

@@ -32,7 +32,7 @@
       >
       <TableSimple
         @rowClick="open"
-        :columns="['id', 'name']"
+        :columns="['index', 'id', 'name']"
         :rows="reportsWithId"
         class="bg-white"
         selectColumn="id"
@@ -101,7 +101,8 @@ export default {
       if (this.reports) {
         let index = 0;
         return this.reports.map((report) => {
-          report.id = index++;
+          report.index = index++;
+          report.id = report.id || report.index;
           return report;
         });
       }
@@ -116,7 +117,7 @@ export default {
     },
     async add() {
       this.error = null;
-      this.reports.push({ name: "new report", sql: "" });
+      this.reports.push({ id: "newid", name: "new report", sql: "" });
       await this.client
         .saveSetting("reports", this.reports)
         .catch((error) => (this.error = error));
@@ -131,7 +132,7 @@ export default {
       this.reload();
     },
     open(row) {
-      this.$router.push({ name: "edit", params: { id: row.id } });
+      this.$router.push({ name: "edit", params: { index: row.index } });
     },
     downloadSelected() {
       window.open(

apps/reports/src/components/ViewEditReport.vue

@@ -16,7 +16,19 @@
       <h2>
         Edit report: {{ id }}<IconAction icon="eye" @click="edit = false" />
       </h2>
-      <InputString id="reportName" v-model="name" label="name" />
+      <InputString
+        id="reportId"
+        v-model="id"
+        label="id"
+        :required="true"
+        description="unique index"
+      />
+      <InputString
+        id="reportName"
+        v-model="name"
+        label="name"
+        description="human readible label"
+      />
       <InputText
         id="reportSql"
         v-model="sql"
@@ -96,13 +108,14 @@ export default {
   },
   props: {
     session: Object,
-    id: String,
+    index: String,
     limit: { type: Number, default: 5 },
   },
   data() {
     return {
       rows: undefined,
       count: null,
+      id: null,
       sql: 'select * from "Pet"',
       name: null,
       parameters: {},
@@ -175,7 +188,11 @@ export default {
       const offset = this.limit * (this.page - 1);
       const result = await request(
         "graphql",
-        `query report($parameters:[MolgenisSettingsInput]) {_reports(id:${this.id},parameters:$parameters,limit:${this.limit},offset:${offset}){data,count}}`,
+        `query report($parameters:[MolgenisSettingsInput]) {_reports(id:"${
+          this.id || this.index
+        }",parameters:$parameters,limit:${
+          this.limit
+        },offset:${offset}){data,count}}`,
         {
           parameters: this.parameterKeyValueMap,
         }
@@ -186,11 +203,20 @@ export default {
       this.count = result._reports.count;
     },
     async save() {
+      if (this.id == null) {
+        this.error = "id is required";
+        return;
+      }
+      if (this.sql == null) {
+        this.error = "sql is required";
+        return;
+      }
       this.succes = null;
       this.error = null;
       const reports = await this.client.fetchSettingValue("reports");
-      reports[this.id].sql = this.sql;
-      reports[this.id].name = this.name;
+      reports[this.index].id = this.id;
+      reports[this.index].sql = this.sql;
+      reports[this.index].name = this.name;
       this.client
         .saveSetting("reports", reports)
         .then((res) => {
@@ -201,9 +227,10 @@ export default {
     },
     async reload() {
       const reports = await this.client.fetchSettingValue("reports");
-      if (reports[this.id]) {
-        this.sql = reports[this.id].sql;
-        this.name = reports[this.id].name;
+      if (reports[this.index]) {
+        this.id = reports[this.index].id;
+        this.sql = reports[this.index].sql;
+        this.name = reports[this.index].name;
       } else {
         this.error = "report not found";
       }

apps/reports/src/main.js

@@ -15,7 +15,7 @@ const router = createRouter({
       props: true,
     },
     {
-      path: "/:id",
+      path: "/:index",
       name: "edit",
       component: ViewReport,
       props: true,

backend/molgenis-emx2-datamodels/src/main/java/org/molgenis/emx2/datamodels/PetStoreLoader.java

@@ -225,9 +225,9 @@ private void loadExampleData(Schema schema) {
         .getMetadata()
         .setSetting(
             "reports",
-            "[{\"id\":0,\"name\":\"pet report\",\"sql\":\"select * from \\\"Pet\\\"\"},"
-                + "{\"id\":1,\"name\":\"pet report with parameters\",\"sql\":\"select * from \\\"Pet\\\" p where p.name=ANY(${name:string_array})\"},"
-                + "{\"id\":2,\"name\":\"jsonb\",\"sql\":\"SELECT jsonb_agg(to_jsonb(\\\"Pet\\\")) AS result FROM \\\"Pet\\\"\"},"
-                + "{\"id\":3,\"name\":\"jsonb rows\",\"sql\":\"SELECT to_jsonb(\\\"Pet\\\") AS result FROM \\\"Pet\\\"\"}]");
+            "[{\"id\":\"report1\",\"name\":\"pet report\",\"sql\":\"select * from \\\"Pet\\\"\"},"
+                + "{\"id\":\"report2\",\"name\":\"pet report with parameters\",\"sql\":\"select * from \\\"Pet\\\" p where p.name=ANY(${name:string_array})\"},"
+                + "{\"id\":\"report3\",\"name\":\"jsonb\",\"sql\":\"SELECT jsonb_agg(to_jsonb(\\\"Pet\\\")) AS result FROM \\\"Pet\\\"\"},"
+                + "{\"id\":\"report4\",\"name\":\"jsonb rows\",\"sql\":\"SELECT to_jsonb(\\\"Pet\\\") AS result FROM \\\"Pet\\\"\"}]");
   }
 }

backend/molgenis-emx2-graphql/src/main/java/org/molgenis/emx2/graphql/GraphqlSchemaFieldFactory.java

@@ -13,10 +13,7 @@
 import graphql.Scalars;
 import graphql.schema.*;
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 import java.util.stream.Stream;
 import org.jooq.JSONB;
 import org.molgenis.emx2.*;
@@ -863,7 +860,7 @@ public GraphQLFieldDefinition schemaReportsField(Schema schema) {
                     GraphQLFieldDefinition.newFieldDefinition()
                         .name(COUNT)
                         .type(Scalars.GraphQLInt)))
-        .argument(GraphQLArgument.newArgument().name(ID).type(Scalars.GraphQLInt))
+        .argument(GraphQLArgument.newArgument().name(ID).type(Scalars.GraphQLString))
         .argument(
             GraphQLArgument.newArgument()
                 .name(PARAMETERS)
@@ -872,20 +869,26 @@ public GraphQLFieldDefinition schemaReportsField(Schema schema) {
         .argument(GraphQLArgument.newArgument().name(OFFSET).type(Scalars.GraphQLInt))
         .dataFetcher(
             dataFetchingEnvironment -> {
-              Integer id = null;
               Map<String, Object> result = new LinkedHashMap<>();
               try {
                 String reportsJson = schema.getMetadata().getSetting("reports");
                 logger.info("REPORT value: " + reportsJson);
                 if (reportsJson != null) {
-                  id = dataFetchingEnvironment.getArgument(ID);
+                  final String id = dataFetchingEnvironment.getArgument(ID);
                   Integer offset = dataFetchingEnvironment.getArgumentOrDefault(OFFSET, 0);
                   Integer limit = dataFetchingEnvironment.getArgumentOrDefault(LIMIT, 10);
                   Map<String, String> parameters =
                       convertKeyValueListToMap(dataFetchingEnvironment.getArgument(PARAMETERS));
                   List<Map<String, Object>> reportList =
                       new ObjectMapper().readValue(reportsJson, List.class);
-                  Map<String, Object> report = reportList.get(id);
+                  Map<String, Object> report =
+                      reportList.stream()
+                          .filter(r -> id.equals(r.get("id")))
+                          .findFirst()
+                          .orElseGet(() -> null);
+                  if (report == null) {
+                    report = reportList.get(Integer.parseInt(id));
+                  }
                   String sql = report.get("sql") + " LIMIT " + limit + " OFFSET " + offset;
                   String countSql =
                       String.format("select count(*) from (%s) as count", report.get("sql"));
@@ -896,7 +899,9 @@ public GraphQLFieldDefinition schemaReportsField(Schema schema) {
                 }
                 return result;
               } catch (Exception e) {
-                throw new MolgenisException("Retrieve of report '" + id + "' failed ", e);
+                throw new MolgenisException(
+                    "Retrieve of report '" + dataFetchingEnvironment.getArgument(ID) + "' failed ",
+                    e);
               }
             })
         .build();

backend/molgenis-emx2-graphql/src/test/java/org/molgenis/emx2/graphql/TestGraphqlSchemaFields.java

@@ -833,12 +833,21 @@ public void testReport() throws IOException {
     schema = database.dropCreateSchema(schemaName);
     PET_STORE.getImportTask(schema, true).run();
     grapql = new GraphqlApiFactory().createGraphqlForSchema(schema, taskService);
-    JsonNode result = execute("{_reports(id:0){data,count}}");
+
+    // by index = 0
+    JsonNode result = execute("{_reports(id:\"0\"){data,count}}");
     assertTrue(result.at("/_reports/data").textValue().contains("pooky"));
     assertEquals(8, result.at("/_reports/count").intValue());
 
     // report 1 has parameters
-    result = execute("{_reports(id:1,parameters:{key:\"name\", value:\"spike\"}){data,count}}");
+    result = execute("{_reports(id:\"1\",parameters:{key:\"name\", value:\"spike\"}){data,count}}");
+    assertTrue(result.at("/_reports/data").textValue().contains("spike"));
+    assertEquals(1, result.at("/_reports/count").intValue());
+
+    // report by id=report1
+    result =
+        execute(
+            "{_reports(id:\"report2\",parameters:{key:\"name\", value:\"spike\"}){data,count}}");
     assertTrue(result.at("/_reports/data").textValue().contains("spike"));
     assertEquals(1, result.at("/_reports/count").intValue());
   }

backend/molgenis-emx2-webapi/src/main/java/org/molgenis/emx2/web/JsonApi.java

@@ -37,9 +37,16 @@ public static void getJsonReport(Context ctx) throws JsonProcessingException {
     Map<String, Object> jsonResponse = new HashMap<>();
 
     for (String reportId : reports.split(",")) {
-      Map<String, Object> reportObject = reportList.get(Integer.parseInt(reportId.trim()));
+
+      Map<String, Object> reportObject =
+          reportList.stream()
+              .filter(r -> reportId.equals(r.get("id")))
+              .findFirst()
+              .orElseGet(() -> null);
+      if (reportObject == null) {
+        reportObject = reportList.get(Integer.parseInt(reportId.trim()));
+      }
       String sql = (String) reportObject.get("sql");
-      String name = (String) reportObject.get("name");
       List<Row> rows = schema.retrieveSql(sql, parameters);
       List<Object> result = new ArrayList<>();
       for (Row row : rows) {
@@ -51,7 +58,7 @@ public static void getJsonReport(Context ctx) throws JsonProcessingException {
         } else {
           result.add(row.getValueMap());
         }
-        jsonResponse.put(name, result);
+        jsonResponse.put(reportId, result);
       }
     }
     if (jsonResponse.size() == 1) {

backend/molgenis-emx2-webapi/src/main/java/org/molgenis/emx2/web/ZipApi.java

@@ -174,21 +174,35 @@ static void getZippedReports(Context ctx) throws IOException {
     }
   }
 
-  static void generateReportsToStore(Context ctx, TableStore store) throws JsonProcessingException {
+  static void generateReportsToStore(
+      @org.jetbrains.annotations.NotNull Context ctx, TableStore store)
+      throws JsonProcessingException {
     String reports = ctx.queryParam("id");
     Schema schema = getSchema(ctx);
     Map<String, ?> parameters = getReportParameters(ctx);
     String reportsJson = schema.getMetadata().getSetting("reports");
     List<Map<String, String>> reportList = new ObjectMapper().readValue(reportsJson, List.class);
     for (String reportId : reports.split(",")) {
-      Map reportObject = reportList.get(Integer.parseInt(reportId));
-      String sql = (String) reportObject.get("sql");
-      String name = (String) reportObject.get("name");
+      // first find report object based on id
+      Optional<Map<String, String>> found =
+          reportList.stream()
+              .filter(reportDefinition -> reportId.equals(reportDefinition.get("id")))
+              .findFirst();
+      Map<String, String> reportObject = null;
+      if (found.isPresent()) {
+        reportObject = found.get();
+      } else {
+        reportObject = reportList.get(Integer.parseInt(reportId));
+      }
+      if (reportObject == null) {
+        throw new MolgenisException("Cannot find report id=" + reportId);
+      }
+      String sql = reportObject.get("sql");
       List<Row> rows = schema.retrieveSql(sql, parameters);
       if (rows.size() > 0) {
-        store.writeTable(name, new ArrayList<>(rows.get(0).getColumnNames()), rows);
+        store.writeTable(reportId, new ArrayList<>(rows.get(0).getColumnNames()), rows);
       } else {
-        store.writeTable(name, new ArrayList<>(), rows);
+        store.writeTable(reportId, new ArrayList<>(), rows);
       }
     }
   }

backend/molgenis-emx2-webapi/src/test/java/org.molgenis.emx2.web/WebApiSmokeTests.java

@@ -174,6 +174,13 @@ public void testReports() throws IOException {
     TableStore store = new TableStoreForCsvInZipFile(zipFile.toPath());
     store.containsTable("pet report");
 
+    // check if 'id' also works
+    zipContents =
+        getContentAsByteArray(ACCEPT_ZIP, "/pet store reports/api/reports/zip?id=report1");
+    zipFile = createTempFile(zipContents, ".zip");
+    store = new TableStoreForCsvInZipFile(zipFile.toPath());
+    store.containsTable("pet report");
+
     // check if reports work with parameters
     zipContents =
         getContentAsByteArray(
@@ -187,29 +194,50 @@ public void testReports() throws IOException {
         getContentAsByteArray(ACCEPT_ZIP, "/pet store reports/api/reports/excel?id=0");
     File excelFile = createTempFile(excelContents, ".xlsx");
     store = new TableStoreForXlsxFile(excelFile.toPath());
-    assertTrue(store.containsTable("pet report"));
+    assertTrue(store.containsTable("0"));
+
+    // check if excel by id also works
+    excelContents =
+        getContentAsByteArray(ACCEPT_ZIP, "/pet store reports/api/reports/excel?id=report1");
+    excelFile = createTempFile(excelContents, ".xlsx");
+    store = new TableStoreForXlsxFile(excelFile.toPath());
+    assertTrue(store.containsTable("report1"));
 
     // check if reports work with parameters
     excelContents =
         getContentAsByteArray(
-            ACCEPT_ZIP, "/pet store reports/api/reports/excel?id=1&name=spike,pooky");
+            ACCEPT_ZIP, "/pet store reports/api/reports/excel?id=report2&name=spike,pooky");
     excelFile = createTempFile(excelContents, ".xlsx");
     store = new TableStoreForXlsxFile(excelFile.toPath());
-    assertTrue(store.containsTable("pet report with parameters"));
+    assertTrue(store.containsTable("report2"));
     assertTrue(excelContents.length > 0);
 
     // test json report api
     String jsonResults =
-        given().sessionId(SESSION_ID).get("/pet store reports/api/reports/json?id=0").asString();
-    assertFalse(jsonResults.contains("pet report"), "single result should not include report name");
+        given()
+            .sessionId(SESSION_ID)
+            .get("/pet store reports/api/reports/json?id=report1")
+            .asString();
+    assertFalse(
+        jsonResults.contains("report1"),
+        "single result should not include report name"); // are we sure about this?
     jsonResults =
         given()
             .sessionId(SESSION_ID)
-            .get("/pet store reports/api/reports/json?id=0,1&name=pooky")
+            .get("/pet store reports/api/reports/json?id=report1,report2&name=pooky")
             .asString();
     assertTrue(
-        jsonResults.contains("pet report"),
+        jsonResults.contains("report1"),
         "multiple results should use the report name to nest results");
+    // check that id is for keys
+    jsonResults =
+        given()
+            .sessionId(SESSION_ID)
+            .get("/pet store reports/api/reports/json?id=report1,report2&name=pooky")
+            .asString();
+    assertTrue(jsonResults.contains("report1"), "should use report id as key");
+    assertTrue(jsonResults.contains("report2"), "should use report id as key");
+
     jsonResults =
         given()
             .sessionId(SESSION_ID)