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

ALS-7867 #62

Open
wants to merge 2 commits into
base: release
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.*;

import java.util.List;

@Controller
@RequestMapping("/dashboard-drawer")
public class DashboardDrawerController {
Expand All @@ -13,7 +15,7 @@ public class DashboardDrawerController {
private DashboardDrawerService dashboardDrawerService;

@GetMapping
public ResponseEntity<DashboardDrawerList> findAll() {
public ResponseEntity<List<DashboardDrawer>> findAll() {
Copy link
Author

Choose a reason for hiding this comment

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

Dumping DashboardDrawerList object in favor of using an actual list

return dashboardDrawerService.findAll().map(ResponseEntity::ok).orElseGet(() -> ResponseEntity.notFound().build());
}

Expand Down

This file was deleted.

Copy link
Author

Choose a reason for hiding this comment

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

Out of scope for this work. I wouldn't build a repository layer here. I would move the repository logic to the dataset repository.

Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package edu.harvard.dbmi.avillach.dictionary.dashboarddrawer;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
Expand All @@ -14,19 +12,14 @@ public class DashboardDrawerRepository {

private final NamedParameterJdbcTemplate template;

private static final Logger log = LoggerFactory.getLogger(DashboardDrawerRepository.class);

@Autowired
public DashboardDrawerRepository(NamedParameterJdbcTemplate template) {
this.template = template;
}

public List<DashboardDrawer> getDashboardDrawerRows() {
String materializedViewSql = """
select * from dictionary_db.dict.dataset_meta_materialized_view dmmv;
""";
public Optional<List<DashboardDrawer>> getDashboardDrawerRows() {
Copy link
Author

Choose a reason for hiding this comment

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

Debug log is too chatty. removing materialized views as they are not in use anyways.


String fallbackSql = """
String sql = """
SELECT d.dataset_id,
MAX(d.full_name) study_fullname,
MAX(d.abbreviation) study_abbreviation,
Expand All @@ -37,24 +30,16 @@ public List<DashboardDrawer> getDashboardDrawerRows() {
MAX(DISTINCT dm.value) FILTER (where dm.key IN ('sponsor')) sponsor
FROM dataset d
JOIN dataset_meta dm ON d.dataset_id = dm.dataset_id
JOIN consent c ON d.dataset_id = c.dataset_id
LEFT JOIN consent c ON d.dataset_id = c.dataset_id
GROUP BY d.dataset_id
""";

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

public Optional<DashboardDrawer> getDashboardDrawerRows(Integer datasetId) {
String materializedViewSql = """
select * from dictionary_db.dict.dataset_meta_materialized_view dmmv where dmmv.dataset_id = :datasetId;
""";
}

String fallbackSql = """
public Optional<DashboardDrawer> getDashboardDrawerRowsByDatasetId(Integer datasetId) {
String sql = """
SELECT d.dataset_id dataset_id,
MAX(d.full_name) study_fullname,
MAX(d.abbreviation) study_abbreviation,
Expand All @@ -72,11 +57,6 @@ public Optional<DashboardDrawer> getDashboardDrawerRows(Integer datasetId) {
MapSqlParameterSource params = new MapSqlParameterSource();
params.addValue("datasetId", datasetId);

try {
return template.query(materializedViewSql, params, new DashboardDrawerRowMapper()).stream().findFirst();
} catch (Exception e) {
log.debug("Materialized view not available, using fallback query. Error: {}", e.getMessage());
return template.query(fallbackSql, params, new DashboardDrawerRowMapper()).stream().findFirst();
}
return template.query(sql, params, new DashboardDrawerRowMapper()).stream().findFirst();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class DashboardDrawerService {
private final String dashboardLayout;

@Autowired
public DashboardDrawerService(DashboardDrawerRepository repository, @Value("${dashboard.layout.type}") String dashboardLayout) {
public DashboardDrawerService(DashboardDrawerRepository repository, @Value("${dashboard.layout.type:default}") String dashboardLayout) {
Copy link
Author

@TDeSain TDeSain Dec 15, 2024

Choose a reason for hiding this comment

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

Since there is no other implementation or requirements for other projects at the moment, will just make this the default layout. I do not like the value to be bdc for obvious reasons.

this.repository = repository;
this.dashboardLayout = dashboardLayout;
}
Expand All @@ -25,13 +25,11 @@ public DashboardDrawerService(DashboardDrawerRepository repository, @Value("${da
*
* @return All Dashboard Instances and their metadata.
*/
public Optional<DashboardDrawerList> findAll() {
if (dashboardLayout.equalsIgnoreCase("bdc")) {
List<DashboardDrawer> records = repository.getDashboardDrawerRows();
return Optional.of(new DashboardDrawerList(records));
public Optional<List<DashboardDrawer>> findAll() {
if (dashboardLayout.equalsIgnoreCase("default")) {
return repository.getDashboardDrawerRows();
}

return Optional.of(new DashboardDrawerList(new ArrayList<>()));
return Optional.of(new ArrayList<>());
}

/**
Expand All @@ -42,8 +40,8 @@ public Optional<DashboardDrawerList> findAll() {
* @return a single Dashboard instance with drawer-specific metadata.
*/
public Optional<DashboardDrawer> findByDatasetId(Integer datasetId) {
if ("bdc".equalsIgnoreCase(dashboardLayout)) {
return repository.getDashboardDrawerRows(datasetId);
if (dashboardLayout.equalsIgnoreCase("default")) {
return repository.getDashboardDrawerRowsByDatasetId(datasetId);
}
return Optional.empty();
}
Expand Down
1 change: 0 additions & 1 deletion src/main/resources/application-bdc.properties
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ server.port=80

dashboard.enable.extra_details=true
dashboard.enable.bdc_hack=true
Copy link
Author

Choose a reason for hiding this comment

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

just use default layout

dashboard.layout.type=bdc

filtering.unfilterable_concepts=stigmatized

Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package edu.harvard.dbmi.avillach.dictionary.dashboarddrawer;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.test.context.ActiveProfiles;

import java.util.List;
import java.util.Optional;

import static org.mockito.Mockito.when;



@SpringBootTest
@ActiveProfiles("test")
class DashboardDrawerControllerTest {

@Autowired
private DashboardDrawerController subject;

@MockBean
private DashboardDrawerService dashboardDrawerService;

@Test
void shouldFindAllDashboardDrawers() {
DashboardDrawer sampleData1 = new DashboardDrawer(
1, // dataset id
"Test Data Set 1", // study full name
"TDS1", // study abbreviation
List.of("group1", "group2"), // consent groups
"Test Study for Unit / Integration Testing", // study summary
List.of("Study Focus One", "Study Focus Two"), // study focus
"Test Study Design", // study design
"Test Study Sponsor" // study sponsor
);

DashboardDrawer sampleData2 = new DashboardDrawer(
2, "Test Data Set 2", "TDS2", List.of("group2"), "Test Study for Unit / Integration Testing", List.of("Study Focus One"),
"Test Study Design", "Test Study Sponsor"
);

when(dashboardDrawerService.findAll()).thenReturn(Optional.of(List.of(sampleData1, sampleData2)));

ResponseEntity<List<DashboardDrawer>> result = subject.findAll();

Assertions.assertEquals(HttpStatus.OK, result.getStatusCode());

List<DashboardDrawer> actualDrawerData = result.getBody();

Assertions.assertNotNull(actualDrawerData, "Expected non-null list of DashboardDrawer");
Assertions.assertEquals(2, actualDrawerData.size(), "Expected two drawers in the result");
}

@Test
void shouldFindDashboardDrawerById() {
DashboardDrawer expectedDrawer = new DashboardDrawer(
1, "Test Data Set 1", "TDS1", List.of("group1", "group2"), "Test Study for Unit / Integration Testing",
List.of("Study Focus One", "Study Focus Two"), "Test Study Design", "Test Study Sponsor"
);

when(dashboardDrawerService.findByDatasetId(1)).thenReturn(Optional.of(expectedDrawer));

ResponseEntity<DashboardDrawer> actualDrawer = subject.findByDatasetId(1);

Assertions.assertEquals(expectedDrawer, actualDrawer.getBody());

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package edu.harvard.dbmi.avillach.dictionary.dashboarddrawer;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.context.DynamicPropertyRegistry;
import org.springframework.test.context.DynamicPropertySource;
import org.testcontainers.containers.PostgreSQLContainer;
import org.testcontainers.junit.jupiter.Container;
import org.testcontainers.junit.jupiter.Testcontainers;
import org.testcontainers.utility.MountableFile;

import java.util.List;
import java.util.Optional;

@Testcontainers
@SpringBootTest
class DashboardDrawerRepositoryTest {
@Autowired
DashboardDrawerRepository subject;

@Container
static final PostgreSQLContainer<?> databaseContainer = new PostgreSQLContainer<>("postgres:16").withReuse(true)
.withCopyFileToContainer(MountableFile.forClasspathResource("seed.sql"), "/docker-entrypoint-initdb.d/seed.sql");

@DynamicPropertySource
static void mySQLProperties(DynamicPropertyRegistry registry) {
registry.add("spring.datasource.url", databaseContainer::getJdbcUrl);
registry.add("spring.datasource.username", databaseContainer::getUsername);
registry.add("spring.datasource.password", databaseContainer::getPassword);
registry.add("spring.datasource.db", databaseContainer::getDatabaseName);
}

@Test
void shouldGetDashboardDrawers() {
Optional<List<DashboardDrawer>> result = subject.getDashboardDrawerRows();

Assertions.assertTrue(result.isPresent(), "Expected dashboard drawers to be present.");
}

@Test
void shouldGetDashboardDrawersByDatasetId() {
int expectedDatasetId = 17;
Optional<DashboardDrawer> result = subject.getDashboardDrawerRowsByDatasetId(expectedDatasetId);

Assertions.assertTrue(result.isPresent(), "Expected a DashboardDrawer to be present");
Assertions.assertEquals(expectedDatasetId, result.get().datasetId(), "Expected the dataset id to be " + expectedDatasetId);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
package edu.harvard.dbmi.avillach.dictionary.dashboarddrawer;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.test.context.ActiveProfiles;

import java.util.List;
import java.util.Optional;

import static org.mockito.Mockito.*;

@SpringBootTest
@ActiveProfiles("test")
class DashboardDrawerServiceTest {

@Autowired
DashboardDrawerService service;

@MockBean
DashboardDrawerRepository repository;

@Test
void shouldFindAllDashboardDrawers() {
// Arrange: Mock the repository to return a sample dataset
when(repository.getDashboardDrawerRows()).thenReturn(Optional.of(List.of(
new DashboardDrawer(1, "Test Data Set 1", "TDS1", List.of("group1", "group2"), "Test Study for Unit / Integration Testing", List.of("Study Focus One", "Study Focus Two"), "Test Study Design", "Test Study Sponsor"),
new DashboardDrawer(2, "Test Data Set 2", "TDS2", List.of("group2"), "Test Study for Unit / Integration Testing", List.of("Study Focus One"), "Test Study Design", "Test Study Sponsor")
)));

// Act: Call the service method
Optional<List<DashboardDrawer>> result = service.findAll();

Assertions.assertTrue(result.isPresent());
List<DashboardDrawer> dashboardDrawers = result.get();
Assertions.assertEquals(2, dashboardDrawers.size());

DashboardDrawer firstDrawer = dashboardDrawers.getFirst();
Assertions.assertEquals(1, firstDrawer.datasetId());
Assertions.assertEquals("Test Data Set 1", firstDrawer.studyFullname());
Assertions.assertEquals("TDS1", firstDrawer.studyAbbreviation());
Assertions.assertEquals(List.of("group1", "group2"), firstDrawer.consentGroups());
Assertions.assertEquals("Test Study for Unit / Integration Testing", firstDrawer.studySummary());
Assertions.assertEquals(List.of("Study Focus One", "Study Focus Two"), firstDrawer.studyFocus());
Assertions.assertEquals("Test Study Design", firstDrawer.studyDesign());
Assertions.assertEquals("Test Study Sponsor", firstDrawer.sponsor());

DashboardDrawer secondDrawer = dashboardDrawers.get(1);
Assertions.assertEquals(2, secondDrawer.datasetId());
Assertions.assertEquals("Test Data Set 2", secondDrawer.studyFullname());
Assertions.assertEquals("TDS2", secondDrawer.studyAbbreviation());
Assertions.assertEquals(List.of("group2"), secondDrawer.consentGroups());
Assertions.assertEquals("Test Study for Unit / Integration Testing", firstDrawer.studySummary());
Assertions.assertEquals(List.of("Study Focus One"), secondDrawer.studyFocus());
Assertions.assertEquals("Test Study Design", secondDrawer.studyDesign());
Assertions.assertEquals("Test Study Sponsor", secondDrawer.sponsor());
}

@Test
void shouldFindADashboardDrawerByDataasetId() {
// Arrange: Mock the repository to return a sample dataset
when(repository.getDashboardDrawerRowsByDatasetId(1)).thenReturn(Optional.of(
new DashboardDrawer(1, "Test Data Set 1", "TDS1", List.of("group1", "group2"), "Test Study for Unit / Integration Testing", List.of("Study Focus One", "Study Focus Two"), "Test Study Design", "Test Study Sponsor")
));

// Act: Call the service method
Optional<DashboardDrawer> result = service.findByDatasetId(1);

Assertions.assertTrue(result.isPresent());
DashboardDrawer drawer = result.get();

Assertions.assertEquals(1, drawer.datasetId());
Assertions.assertEquals("Test Data Set 1", drawer.studyFullname());
Assertions.assertEquals("TDS1", drawer.studyAbbreviation());
Assertions.assertEquals(List.of("group1", "group2"), drawer.consentGroups());
Assertions.assertEquals("Test Study for Unit / Integration Testing", drawer.studySummary());
Assertions.assertEquals(List.of("Study Focus One", "Study Focus Two"), drawer.studyFocus());
Assertions.assertEquals("Test Study Design", drawer.studyDesign());
Assertions.assertEquals("Test Study Sponsor", drawer.sponsor());

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package edu.harvard.dbmi.avillach.dictionary.dashboarddrawer;

import org.junit.jupiter.api.Test;

import java.util.List;

import static org.junit.jupiter.api.Assertions.assertEquals;

class DashboardDrawerTest {

@Test
void testDashboardDrawerInstantiation() {
DashboardDrawer dashboardDrawer = new DashboardDrawer(
1, "Full Name", "Abbr", List.of("Consent1", "Consent2"), "Summary", List.of("Focus1", "Focus2"), "Design", "Sponsor"
);

assertEquals(1, dashboardDrawer.datasetId());
assertEquals("Full Name", dashboardDrawer.studyFullname());
assertEquals("Abbr", dashboardDrawer.studyAbbreviation());
assertEquals(List.of("Consent1", "Consent2"), dashboardDrawer.consentGroups());
assertEquals("Summary", dashboardDrawer.studySummary());
assertEquals(List.of("Focus1", "Focus2"), dashboardDrawer.studyFocus());
assertEquals("Design", dashboardDrawer.studyDesign());
assertEquals("Sponsor", dashboardDrawer.sponsor());
}
}
Loading