[ALS-7694] Add DashboardDrawer feature with controller, service, and repository #55

TDeSain · 2024-11-18T13:37:30Z

Introduce DashboardDrawerController, DashboardDrawerService, and DashboardDrawerRepository classes for dashboard drawer functionality. Include methods to fetch drawer data for datasets using materialized views.

- Introduce DashboardDrawerController for drawer-related endpoints - Implement DashboardDrawerService to handle drawer logic - Create DashboardDrawerRepository for database interactions - Define DashboardDrawer and DashboardDrawerList records - Add DashboardDrawerRowMapper for result set mapping - Update DashboardRepository SQL to include dataset_id - Modify application properties to include dashboard layout configuration

- Added `dashboard.enable.bdc_hack` property - Added `dashboard.layout.type` property - Moved `filtering.unfilterable_concepts` property - Ensured newline at end of file

TDeSain · 2024-11-20T13:04:35Z

src/main/java/edu/harvard/dbmi/avillach/dictionary/dashboard/DashboardRepository.java

@@ -52,6 +52,7 @@ public List<Map<String, String>> getHackyBDCRows() {
        String sql =
            """
                    SELECT
+                        dataset.dataset_id as dataset_id,


Can use the dataset table's primary key the return object to make downstream processes more efficient

TDeSain · 2024-11-20T13:57:37Z

...ain/java/edu/harvard/dbmi/avillach/dictionary/dashboarddrawer/DashboardDrawerRepository.java

+    }
+
+    public List<DashboardDrawer> getDashboardDrawerRows() {
+        String materializedViewSql = """


Materialized views are pretty powerful tools. I am not in love with how Postgres handles them as you have to build logic to use them unlike Oracles Query Rewrite abilities.

https://docs.oracle.com/cd/B10501_01/server.920/a96520/qr.htm

This query is not expensive so I am not worried about needing this unless we scale number of studies and retrieved metadata. Can remove materialized view logic if needed.

Would make the query much more dynamic / possibly stored procedure. Aggregation gets expensive.

Let's talk about this in OH. I clearly don't use views often enough and I want to learn more about them from you.

TDeSain · 2024-11-20T13:59:19Z

...ain/java/edu/harvard/dbmi/avillach/dictionary/dashboarddrawer/DashboardDrawerRepository.java

+        }
+    }
+
+    public List<DashboardDrawer> getDashboardDrawerRows(Integer datasetId) {


Same as other query but this will find a specific record based on the datasetId

TDeSain · 2024-11-20T14:01:09Z

...main/java/edu/harvard/dbmi/avillach/dictionary/dashboarddrawer/DashboardDrawerRowMapper.java

+import java.util.List;
+
+public class DashboardDrawerRowMapper implements RowMapper<DashboardDrawer> {
+


I would at least make an abstract class for this that I could extend to fulfill multiple Dashboard Layouts.

Should probably do this for other Classes such as Dashboard.

TDeSain · 2024-11-20T14:03:08Z

...main/java/edu/harvard/dbmi/avillach/dictionary/dashboarddrawer/DashboardDrawerRowMapper.java

+        } else {
+            Object[] arrayContents = (Object[]) sqlArray.getArray();
+            // Check if the array contains a single empty value
+            if (arrayContents.length == 1 && "".equals(arrayContents[0])) {


This is a bit hacky but sqlArray returns an single empty value for empty values in the db. Which makes sense as empty values can have meaning. For this component they do not.

src/main/resources/application-bdc.properties

Luke-Sikina · 2024-11-20T14:34:53Z

...ain/java/edu/harvard/dbmi/avillach/dictionary/dashboarddrawer/DashboardDrawerController.java

@@ -0,0 +1,24 @@
+package edu.harvard.dbmi.avillach.dictionary.dashboarddrawer;


What's the difference between edu.harvard.dbmi.avillach.dictionary.dashboarddrawer and edu.harvard.dbmi.avillach.dictionary.dashboard as far as business purpose goes? Are these legitimately different domains, or is this a BDC specific implementation of the same domain?

Yea I was in the same boat at the beginning and was trying to fit it into to the Dashboard thus why it was basically a replication of it until changed the class names.

I think I should follow up on this ticket with Cleaning up some of the dashboard method and looking at how this integrates with the dashboard more closely.

From more of a backend mindset I think it should relate to a dataset endpoint and not a feature endpoint.

Luke-Sikina · 2024-11-20T14:42:01Z

...ain/java/edu/harvard/dbmi/avillach/dictionary/dashboarddrawer/DashboardDrawerController.java

+
+    @GetMapping("/{id}")
+    public ResponseEntity<DashboardDrawer> findByDatasetId(@PathVariable Integer id) {
+        return ResponseEntity.ok(dashboardDrawerService.findByDatasetId(id));


This should 404 if the dataset DNE. You can do this cleanly by expressing the nullable DashboardDrawer as an Optional<DashboardDrawer> and then mapping that to a response entity here.

Luke-Sikina · 2024-11-20T14:42:33Z

src/main/java/edu/harvard/dbmi/avillach/dictionary/dashboarddrawer/DashboardDrawerList.java

+
+import java.util.List;
+
+public record DashboardDrawerList(List<DashboardDrawer> dashboardDrawerList) {


You made yourself a note to get rid of this record and just return a list. I agree with that note.

I made a comment that DashboardDrawerRecord was redundant as it's already a record.

DashboardDrawerList should be a list of DashboardDrawer as that is the collection type of this record class.

DashboardDrawer is just a record.

Luke-Sikina · 2024-11-20T14:43:36Z

...ain/java/edu/harvard/dbmi/avillach/dictionary/dashboarddrawer/DashboardDrawerRepository.java

+    }
+
+    public List<DashboardDrawer> getDashboardDrawerRows() {
+        String materializedViewSql = """


Let's talk about this in OH. I clearly don't use views often enough and I want to learn more about them from you.

Luke-Sikina · 2024-11-20T14:44:11Z

...ain/java/edu/harvard/dbmi/avillach/dictionary/dashboarddrawer/DashboardDrawerRepository.java

+        try {
+            return template.query(materializedViewSql, new DashboardDrawerRowMapper());
+        } catch (Exception e) {
+            log.debug("Materialized view not available, using fallback query. Error: {}", e.getMessage());


Why can't we make it so this view always exists?

https://www.postgresql.org/docs/current/rules-materializedviews.html

Materialized views are a bit different then views. As in they are stored on disk ( in memory on some DBMS ). They are basically a point in time view of a table. But they need to be managed and refreshed.

Have not use Postgres in my checkered past but they were a Saint when it came to Oracle. Oracle has query rewrite which makes it much easier to manage. From what I see so far in Postgres we will need to write logic blocks like this or queries will fail if Materialized view is being rewritten.

Query rewrite would fall back on the select statement used to create the materialized view if the view doesn't exist or is being refreshed.

Luke-Sikina · 2024-11-20T14:44:45Z

...ain/java/edu/harvard/dbmi/avillach/dictionary/dashboarddrawer/DashboardDrawerRepository.java

+        }
+    }
+
+    public List<DashboardDrawer> getDashboardDrawerRows(Integer datasetId) {


This is selecting a specific dataset ID. Why return a list when it's always either 1 or 0 elements?

Luke-Sikina · 2024-11-20T14:46:52Z

src/main/java/edu/harvard/dbmi/avillach/dictionary/dashboarddrawer/DashboardDrawerService.java

+            }
+        }
+
+        return new DashboardDrawer(-1, "", "", new ArrayList<>(), "", new ArrayList<>(), "", "");


I don't like this zero value, and I want to understand more about the API requirements justify this.

src/main/resources/application-bdc.properties

- Updated SQL query to include 'program_name' field. - Joined 'dataset_meta' table for 'program_name' metadata. - Added 'program_name' to the column list in DashboardService.

- Refactor `DashboardDrawerService` methods to return `Optional` - Update controller methods to handle `Optional` results with appropriate HTTP responses - Modify repository methods to return `Optional` for single item queries - Ensure consistency and error handling improvements across the service and controller layers

TDeSain self-assigned this Nov 18, 2024

TDeSain added the enhancement New feature or request label Nov 18, 2024

JamesPeck changed the title ~~Add DashboardDrawer feature with controller, service, and repository~~ [ALS-7694] Add DashboardDrawer feature with controller, service, and repository Nov 19, 2024

TDeSain force-pushed the ALS-7694 branch from cbcd311 to 194f350 Compare November 19, 2024 20:41

TDeSain force-pushed the ALS-7694 branch from d312705 to 43280f4 Compare November 19, 2024 23:59

TDeSain added 3 commits November 19, 2024 19:00

Merge branch 'refs/heads/release' into ALS-7694

fe86e5b

Update application-bdc.properties with dashboard and filtering configs

f89fa61

- Added `dashboard.enable.bdc_hack` property - Added `dashboard.layout.type` property - Moved `filtering.unfilterable_concepts` property - Ensured newline at end of file

Update application-bdc.properties with dashboard and filtering configs

82303a1

- Added `dashboard.enable.bdc_hack` property - Added `dashboard.layout.type` property - Moved `filtering.unfilterable_concepts` property - Ensured newline at end of file

TDeSain commented Nov 20, 2024

View reviewed changes

TDeSain requested review from Luke-Sikina, Gcolon021 and JamesPeck November 20, 2024 14:11

Luke-Sikina reviewed Nov 20, 2024

View reviewed changes

TDeSain added 3 commits November 20, 2024 11:54

Add 'program_name' column to Dashboard repository

d5706f9

- Updated SQL query to include 'program_name' field. - Joined 'dataset_meta' table for 'program_name' metadata. - Added 'program_name' to the column list in DashboardService.

Merge remote-tracking branch 'refs/remotes/origin/release' into ALS-7694

6ad0322

TDeSain merged commit 60d3904 into release Nov 20, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ALS-7694] Add DashboardDrawer feature with controller, service, and repository #55

[ALS-7694] Add DashboardDrawer feature with controller, service, and repository #55

TDeSain commented Nov 18, 2024

TDeSain Nov 20, 2024

TDeSain Nov 20, 2024

Luke-Sikina Nov 20, 2024

TDeSain Nov 20, 2024

TDeSain Nov 20, 2024

TDeSain Nov 20, 2024

Luke-Sikina Nov 20, 2024

TDeSain Nov 20, 2024

Luke-Sikina Nov 20, 2024

Luke-Sikina Nov 20, 2024

TDeSain Nov 20, 2024

Luke-Sikina Nov 20, 2024

Luke-Sikina Nov 20, 2024

TDeSain Nov 20, 2024

Luke-Sikina Nov 20, 2024

Luke-Sikina Nov 20, 2024

		import java.util.List;

		public class DashboardDrawerRowMapper implements RowMapper<DashboardDrawer> {

		@@ -0,0 +1,24 @@
		package edu.harvard.dbmi.avillach.dictionary.dashboarddrawer;


		import java.util.List;

		public record DashboardDrawerList(List<DashboardDrawer> dashboardDrawerList) {

[ALS-7694] Add DashboardDrawer feature with controller, service, and repository #55

[ALS-7694] Add DashboardDrawer feature with controller, service, and repository #55

Conversation

TDeSain commented Nov 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment