From cbe080bcdc4477bc1f5f8ceeff27abbf0bb94528 Mon Sep 17 00:00:00 2001 From: Kevin Day Date: Thu, 25 Apr 2024 12:14:57 -0500 Subject: [PATCH 1/2] Issue 54: Add custom filter for embargo type by list. There is existing (dead) code for this already. Re-vitalize the dead code, clean it up, and get it working. The `embargoTypes.name` SQL in the back-end operates differently in that it is not involved in part of a column name and is exclusively for filtering. This changes the nature of the query and I opted to go with sub-queries for selecting the IDs because I can design it this way and avoid `LEFT JOIN` (or `INNER JOIN`). This has not seen extensive performance testing but this new SQL query conditions should be run through some performance testing. I went with `Embargo Type` for the new custom filter because the existing code used that name. A custom filter `uniqueEmbargoType` is created because the `unique`/`uniq` angularjs is not available. Using `unique` or `uniq` yields an error like this: ``` Unknown provider: uniqueFilterProvider <- uniqueFilter ``` Using the custom filter (`uniqueEmbargoType`) avoids having to pass the additional ` | filter:{isActive: true}` and similar. This can be done and is done within the same filter. There are no existing filter unit tests to base off of. I may follow up with a commit that adds filter unit tests after reviewing other angularjs projects where we have implemented a filter unit test. The addition of the `Embargo Type` causes a test in the `SystemDataLoaderTest` class to fail due to a hard-coded value representing all of the default filters. This has been updated to include the new filter in the total. The UI has an existing filter view, but it was slightly tweak that filter view to address problems in the user experience. I opted to make its design more closely match that of the Status filter (which goes in line with the issue request using Status filter as an example). --- .../model/repo/impl/SubmissionRepoImpl.java | 44 +++++++------------ ...YSTEM_Default_Submission_List_Columns.json | 12 +++++ .../webapp/app/filters/uniqueEmbargoType.js | 16 +++++++ .../webapp/app/resources/styles/sass/app.scss | 4 ++ .../furtherFilterByEmbargoType.html | 32 +++++++------- .../vireo/service/SystemDataLoaderTest.java | 2 +- 6 files changed, 65 insertions(+), 45 deletions(-) create mode 100644 src/main/webapp/app/filters/uniqueEmbargoType.js diff --git a/src/main/java/org/tdl/vireo/model/repo/impl/SubmissionRepoImpl.java b/src/main/java/org/tdl/vireo/model/repo/impl/SubmissionRepoImpl.java index 79fb337c0c..3564b4d4a2 100644 --- a/src/main/java/org/tdl/vireo/model/repo/impl/SubmissionRepoImpl.java +++ b/src/main/java/org/tdl/vireo/model/repo/impl/SubmissionRepoImpl.java @@ -740,43 +740,31 @@ public int compare(SubmissionListColumn svc1, SubmissionListColumn svc2) { break; case "embargoTypes.name": - sqlBuilder = new StringBuilder() - .append("\nLEFT JOIN") - .append("\n (SELECT e.id, e.name, semt.submission_id") - .append("\n FROM embargo e") - .append("\n LEFT JOIN submission_embargo_types semt") - .append("\n ON semt.embargo_types_id=e.id) embs") - .append("\n ON embs.submission_id=s.id"); - - sqlJoinsBuilder.append(sqlBuilder); - sqlCountSelectBuilder.append(sqlBuilder); - - if (submissionListColumn.getSortOrder() > 0) { - setColumnOrdering(submissionListColumn.getSort(), sqlAliasBuilders, sqlOrderBysBuilder, "embs.name"); - } - - for (String filterString : submissionListColumn.getFilters()) { + // This is not a select column but is instead only a custom filter. + if (submissionListColumn.getFilters().size() > 0) { sqlBuilder = new StringBuilder(); + sqlBuilder.append("s.id IN (SELECT submission_id FROM submission_field_values WHERE field_values_id IN (select id FROM field_value WHERE field_predicate_id IN (SELECT id FROM field_predicate WHERE value IN ('default_embargos', 'proquest_embargos')) and ("); - sqlBuilder.append(" embs").append(".name = '").append(filterString).append("'"); + // Note that the OR query is used inside the column, represented by both default_embargos and proquest_embargos. + boolean hasNone = false; + for (String filterString : submissionListColumn.getFilters()) { + sqlBuilder.append(" value = '").append(escapeString(filterString, false)).append("' OR"); - if (filterString.equals("None")) { - sqlWhereBuilderList.add(sqlBuilder); - sqlBuilder = new StringBuilder(); - sqlBuilder.append("embs.id IS NULL"); + if ("None".equals(filterString)) { + hasNone = true; + } + } + sqlBuilder.setLength(sqlBuilder.length() - 3); + sqlBuilder.append(")))"); + + if (hasNone) { + sqlBuilder.append(" OR s.id NOT IN (SELECT submission_id FROM submission_field_values WHERE field_values_id IN (SELECT id FROM field_value WHERE field_predicate_id IN (SELECT id FROM field_predicate WHERE value IN ('default_embargos', 'proquest_embargos'))))"); } sqlWhereBuilderList.add(sqlBuilder); getFromBuildersMap(sqlCountWhereFilterBuilders, "embargoTypes.name").add(sqlBuilder); } - // all column search filter - for (String filterString : allColumnSearchFilters) { - sqlBuilder = new StringBuilder(); - sqlBuilder.append("LOWER(embs").append(".name) LIKE '%").append(escapeString(filterString)).append("%'"); - sqlAllColumnsWhereBuilderList.add(sqlBuilder); - } - break; case "lastEvent": diff --git a/src/main/resources/submission_list_columns/SYSTEM_Default_Submission_List_Columns.json b/src/main/resources/submission_list_columns/SYSTEM_Default_Submission_List_Columns.json index 981b8ef6cd..1a892acab0 100644 --- a/src/main/resources/submission_list_columns/SYSTEM_Default_Submission_List_Columns.json +++ b/src/main/resources/submission_list_columns/SYSTEM_Default_Submission_List_Columns.json @@ -197,5 +197,17 @@ "inputType": { "name": "INPUT_TEXT" } + }, + { + "title": "Embargo Type", + "sort": "NONE", + "valuePath": [ + "embargoTypes", + "name" + ], + "status": null, + "inputType": { + "name": "INPUT_TEXT" + } } ] diff --git a/src/main/webapp/app/filters/uniqueEmbargoType.js b/src/main/webapp/app/filters/uniqueEmbargoType.js new file mode 100644 index 0000000000..95fffe5d97 --- /dev/null +++ b/src/main/webapp/app/filters/uniqueEmbargoType.js @@ -0,0 +1,16 @@ +vireo.filter('uniqueEmbargoType', function() { + return function(initialValues, isActive) { + var matched = []; + var newValues = []; + if (typeof initialValues === "object") { + angular.forEach(initialValues, function(embargo) { + if (matched.includes(embargo.name)) return; + if (embargo.isActive !== isActive) return; + + matched.push(embargo.name); + newValues.push(embargo); + }); + } + return newValues; + }; +}); diff --git a/src/main/webapp/app/resources/styles/sass/app.scss b/src/main/webapp/app/resources/styles/sass/app.scss index 26835eb8c2..f22e999bfc 100644 --- a/src/main/webapp/app/resources/styles/sass/app.scss +++ b/src/main/webapp/app/resources/styles/sass/app.scss @@ -524,6 +524,7 @@ sidebox ul.sidebox-list input { padding: 15px; } +.sidebox-body ul.sidebox-list.embargotype-list, .sidebox-body ul.sidebox-list.status-list { padding: 0px; } @@ -1560,6 +1561,7 @@ input[type=color]:focus { text-decoration: underline; } +.further-filter-by .embargotype-category, .further-filter-by .status-category { background: #CCCCCC; display: inline-block; @@ -1572,6 +1574,7 @@ input[type=color]:focus { .further-filter-by .document-type-list, .further-filter-by .inactive-filters-sub-list, .further-filter-by .organization-by-category-sub-list, +.further-filter-by .embargotype-list, .further-filter-by .status-list { max-height: 0; overflow: hidden; @@ -1587,6 +1590,7 @@ input[type=color]:focus { .further-filter-by .document-type-list.expanded, .further-filter-by .inactive-filters-sub-list.expanded, .further-filter-by .organization-by-category-sub-list.expanded, +.further-filter-by .embargotype-list.expanded, .further-filter-by .status-list.expanded { max-height: 1000px; -webkit-transition: max-height 0.5s ease-in; diff --git a/src/main/webapp/app/views/sideboxes/furtherFilterBy/furtherFilterByEmbargoType.html b/src/main/webapp/app/views/sideboxes/furtherFilterBy/furtherFilterByEmbargoType.html index 4abe548f68..1af4109c14 100644 --- a/src/main/webapp/app/views/sideboxes/furtherFilterBy/furtherFilterByEmbargoType.html +++ b/src/main/webapp/app/views/sideboxes/furtherFilterBy/furtherFilterByEmbargoType.html @@ -1,17 +1,17 @@ -
- - - - + +
diff --git a/src/test/java/org/tdl/vireo/service/SystemDataLoaderTest.java b/src/test/java/org/tdl/vireo/service/SystemDataLoaderTest.java index 50232fbe9d..959cb90bde 100644 --- a/src/test/java/org/tdl/vireo/service/SystemDataLoaderTest.java +++ b/src/test/java/org/tdl/vireo/service/SystemDataLoaderTest.java @@ -492,7 +492,7 @@ private void assertSubmissionStatusTransitions(String[] expected, SubmissionStat } private void assertSubmissionListColumn(boolean isReload) { - assertEquals(59, submissionListColumnRepo.count(), + assertEquals(60, submissionListColumnRepo.count(), isReload ? "Incorrect number of submissionListColumn found after reload" : "Incorrect number of submissionListColumn found"); From 42f659ce889eb734599ca29e82646035add814dc Mon Sep 17 00:00:00 2001 From: Kevin Day Date: Thu, 25 Apr 2024 13:50:43 -0500 Subject: [PATCH 2/2] Issue 54: Add unit test for Embargo Type filter. This adds the filter (in the context of AngularJS) for the Embargo Type. --- .../unit/filters/uniqueEmbargoTypeTest.js | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 src/main/webapp/tests/unit/filters/uniqueEmbargoTypeTest.js diff --git a/src/main/webapp/tests/unit/filters/uniqueEmbargoTypeTest.js b/src/main/webapp/tests/unit/filters/uniqueEmbargoTypeTest.js new file mode 100644 index 0000000000..9974475e2c --- /dev/null +++ b/src/main/webapp/tests/unit/filters/uniqueEmbargoTypeTest.js @@ -0,0 +1,101 @@ +describe("filter: uniqueEmbargoType", function () { + var $scope, MockedUser, filter; + + var initializeVariables = function () { + inject(function (_$q_) { + $q = _$q_; + + MockedUser = new mockUser($q); + }); + }; + + var initializeFilter = function (settings) { + inject(function (_$filter_, _$rootScope_) { + $scope = _$rootScope_.$new(); + + filter = _$filter_("uniqueEmbargoType"); + }); + }; + + beforeEach(function () { + module("core"); + module("vireo"); + module("mock.user", function ($provide) { + var User = function () { + return MockedUser; + }; + $provide.value("User", User); + }); + module("mock.userService"); + + installPromiseMatchers(); + initializeVariables(); + initializeFilter(); + }); + + afterEach(function () { + $scope.$destroy(); + }); + + describe("Is the filter", function () { + it("defined", function () { + expect(filter).toBeDefined(); + }); + }); + + describe("Does the filter", function () { + it("return empty array on empty input", function () { + var result; + + result = filter("", false); + + expect(result).toEqual([]); + + result = filter("", true); + + expect(result).toEqual([]); + + result = filter(null, false); + + expect(result).toEqual([]); + + result = filter(null, true); + + expect(result).toEqual([]); + + result = filter([], false); + + expect(result).toEqual([]); + + result = filter([], true); + + expect(result).toEqual([]); + + result = filter({}, false); + + expect(result).toEqual([]); + + result = filter({}, true); + + expect(result).toEqual([]); + }); + + it("return array with duplicate removed for active", function () { + var result; + var duplicated = [ dataEmbargo1, dataEmbargo2, dataEmbargo1, dataEmbargo3 ]; + + result = filter(duplicated, true); + + expect(result.length).toBe(2); + }); + + it("return array with duplicate removed for inactive", function () { + var result; + var duplicated = [ dataEmbargo3, dataEmbargo2, dataEmbargo3, dataEmbargo1 ]; + + result = filter(duplicated, false); + + expect(result.length).toBe(1); + }); + }); +});