From 8bc9066947f26df846e0a5bfba936c31762138f0 Mon Sep 17 00:00:00 2001 From: Anssi Kinnunen Date: Fri, 3 Nov 2023 14:17:58 +0200 Subject: [PATCH 1/2] fix: loosen application duo schema and hide duos when disabled by config :application/duo :duo/codes is now always added to application base view (when provided by draft-saved event), but schema expected :duo/matches to exist if :application/duo was present. The specific scenario for this regression happened when :enable-duo config was off, and duo codes were previously recorded in draft-saved event(s). All :application/duo keys are now optional. Application duos are hidden during enrichment if disabled by :enable-duo configuration, so that pdf rendering and other parts do not display them. Previously application base view used config, but this was unintentionally against how application-view documented itself as pure function. --- src/clj/rems/api/schema.clj | 17 +++++----- src/clj/rems/application/model.clj | 41 +++++++++++++++---------- test/clj/rems/api/test_applications.clj | 13 ++++++-- 3 files changed, 45 insertions(+), 26 deletions(-) diff --git a/src/clj/rems/api/schema.clj b/src/clj/rems/api/schema.clj index 3ff7d9301..aa7ada5d9 100644 --- a/src/clj/rems/api/schema.clj +++ b/src/clj/rems/api/schema.clj @@ -218,6 +218,15 @@ (s/defschema Permissions #{(apply s/enum (conj commands/command-names :see-everything))}) +(s/defschema DuoCodeMatch + {:duo/id s/Str + :duo/shorthand s/Str + :duo/label {s/Keyword s/Any} + :resource/id s/Int + :duo/validation {:validity s/Keyword + (s/optional-key :errors) [{:type s/Keyword + s/Keyword s/Any}]}}) + (s/defschema Application {:application/id s/Int :application/external-id (rjs/field @@ -267,13 +276,7 @@ (s/optional-key :entitlement/end) (s/maybe DateTime) (s/optional-key :application/votes) {schema-base/UserId s/Str} (s/optional-key :application/duo) {(s/optional-key :duo/codes) [schema-base/DuoCodeFull] - :duo/matches [{:duo/id s/Str - :duo/shorthand s/Str - :duo/label {s/Keyword s/Any} - :resource/id s/Int - :duo/validation {:validity s/Keyword - (s/optional-key :errors) [{:type s/Keyword - s/Keyword s/Any}]}}]}}) + (s/optional-key :duo/matches) [DuoCodeMatch]}}) (s/defschema ApplicationRaw (-> Application diff --git a/src/clj/rems/application/model.clj b/src/clj/rems/application/model.clj index dffdc19d5..aec112b53 100644 --- a/src/clj/rems/application/model.clj +++ b/src/clj/rems/application/model.clj @@ -2,7 +2,7 @@ (:require [clojure.set :as set] [clojure.test :refer [deftest is testing]] [com.rpl.specter :refer [ALL transform select]] - [medley.core :refer [dissoc-in distinct-by filter-vals find-first map-vals update-existing update-existing-in]] + [medley.core :refer [assoc-some dissoc-in distinct-by filter-vals find-first map-vals update-existing update-existing-in]] [rems.application.events :as events] [rems.application.master-workflow :as master-workflow] [rems.common.application-util :refer [applicant-and-members can-redact-attachment? is-applying-user?]] @@ -511,20 +511,29 @@ :duo/restrictions (:restrictions dataset-code)}] nil)})) -(defn- enrich-application-duo-matches [application get-config] - (if-not (:enable-duo (get-config)) +(defn- get-duo-matches [application] + (for [resource (:application/resources application) + dataset-code (get-in resource [:resource/duo :duo/codes]) + :let [duo-id (:id dataset-code) + query-code (->> (get-in application [:application/duo :duo/codes]) + (find-first (comp #{duo-id} :id)))]] + {:duo/id duo-id + :duo/shorthand (:shorthand dataset-code) + :duo/label (:label dataset-code) + :resource/id (:resource/id resource) + :duo/validation (validate-duo-match dataset-code query-code resource)})) + +(defn- hide-duos-if-not-enabled [application get-config] + (if (:enable-duo (get-config)) application - (let [duos (->> application :application/duo :duo/codes) - matches (for [resource (:application/resources application) - res-duo (-> resource :resource/duo :duo/codes) - :let [app-duo (find-first #(= (:id res-duo) (:id %)) duos)]] - {:duo/id (:id res-duo) - :duo/shorthand (:shorthand res-duo) - :duo/label (:label res-duo) - :resource/id (:resource/id resource) - :duo/validation (validate-duo-match res-duo app-duo resource)})] - (-> application - (assoc-in [:application/duo :duo/matches] matches))))) + (-> application + (dissoc :application/duo) + (update :application/resources (partial mapv #(dissoc % :resource/duo)))))) + +(defn- enrich-duos [application] + (-> application + (update-existing-in [:application/duo :duo/codes] duo/enrich-duo-codes) + (update-existing :application/duo assoc-some :duo/matches (seq (get-duo-matches application))))) (defn- enrich-workflow-licenses [application get-workflow] (let [wf (get-workflow (get-in application [:application/workflow :workflow/id]))] @@ -813,8 +822,8 @@ enrich-field-visible ; uses enriched answers set-application-description ; uses enriched answers (update :application/resources enrich-resources get-catalogue-item) - (enrich-application-duo-matches get-config) ; uses enriched resources - (update-existing-in [:application/duo :duo/codes] duo/enrich-duo-codes) + (hide-duos-if-not-enabled get-config) + enrich-duos ; uses enriched resources (enrich-workflow-licenses get-workflow) (update :application/licenses enrich-licenses get-license) (update :application/events (partial mapv #(enrich-event % get-user get-catalogue-item))) diff --git a/test/clj/rems/api/test_applications.clj b/test/clj/rems/api/test_applications.clj index 2902a9b5b..b5154a3e3 100644 --- a/test/clj/rems/api/test_applications.clj +++ b/test/clj/rems/api/test_applications.clj @@ -2564,6 +2564,7 @@ cat1 (test-helpers/create-catalogue-item! {:workflow-id wfid :resource-id res1}) cat2 (test-helpers/create-catalogue-item! {:workflow-id wfid :resource-id res2}) app-id (test-helpers/create-application! {:actor applicant-id :catalogue-item-ids [cat1 cat2]})] + (testing "save partially valid duo codes" (is (= {:success true} (send-command applicant-id @@ -2620,6 +2621,7 @@ :validity "duo/not-compatible"}}]} (-> (get-application-for-user app-id applicant-id) :application/duo)))) + (testing "save fully valid duo codes" (is (= {:success true} (send-command applicant-id @@ -2676,7 +2678,13 @@ :duo/shorthand "DS" :duo/validation {:errors [] :validity "duo/compatible"}}]} (-> (get-application-for-user app-id applicant-id) - :application/duo)))))))) + :application/duo)))) + + (testing "duo codes are not enriched when config :enable-duo is false" + (with-redefs [rems.config/env (assoc rems.config/env :enable-duo false)] + (let [application (get-application-for-user app-id applicant-id)] + (is (nil? (:application/duo application))) + (is (empty? (keep :resource/duo (:application/resources application))))))))))) (deftest test-application-raw (let [api-key "42" @@ -2816,8 +2824,7 @@ :event/actor-attributes {:userid "alice" :name "Alice Applicant" :nickname "In Wonderland" :email "alice@example.com" :organizations [{:organization/id "default"}] :researcher-status-by "so"} :application/field-values [{:form form-id :field "field-1" :value "raw test"} {:form form-id :field "att" :value (str att-id)}] - :event/visibility "visibility/public"}] - :application/duo {:duo/matches []}} + :event/visibility "visibility/public"}]} (-> (api-call :get (str "/api/applications/" app-id "/raw") nil api-key reporter) ;; event ids are unpredictable From 7f88494a1c8d87ffd989900c3dbc8bef7447ee60 Mon Sep 17 00:00:00 2001 From: Anssi Kinnunen Date: Fri, 3 Nov 2023 16:34:49 +0200 Subject: [PATCH 2/2] release: v2.34.2 --- CHANGELOG.md | 7 ++++++- project.clj | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66eb17fdf..1bf8efab1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,12 @@ have notable changes. ## Unreleased -Changes since v2.34.1 +Changes since v2.34.2 + +## v2.34.2 "Santakatu +2" 2023-11-03 + +### Fixes +- DUO codes in draft saved event no longer cause schema validation error when `:enable-duo` config is false. ## v2.34.1 "Santakatu +1" 2023-11-03 diff --git a/project.clj b/project.clj index 08697c6e0..e194a8e53 100644 --- a/project.clj +++ b/project.clj @@ -1,4 +1,4 @@ -(defproject rems "2.34.1" +(defproject rems "2.34.2" :description "Resource Entitlement Management System is a tool for managing access rights to resources, such as research datasets." :url "https://github.com/CSCfi/rems"