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

feat: Introduce study status and add closing state #634

Merged
merged 24 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
5c0f04f
chore: translation
johannesvedder May 25, 2024
3cf8804
feat: deprecate published for study_status
johannesvedder May 25, 2024
29310b7
chore: create migration temporarily
johannesvedder May 26, 2024
9aba30c
fix: improve status transition rules
johannesvedder May 26, 2024
15a72e5
feat: study close button
johannesvedder Jun 7, 2024
881f856
chore: format
johannesvedder Jun 7, 2024
acdbb1c
chore: copy migration to schema
johannesvedder Jun 10, 2024
aed1eff
fix: merge dashboard refactor pr
johannesvedder Jun 11, 2024
051552d
chore: fix seed
johannesvedder Jun 11, 2024
dbd1810
fix: study visibility policy no longer uses published field
johannesvedder Jun 11, 2024
1ce2f78
fix: potential isClosed error
johannesvedder Jun 11, 2024
7dcc928
fix: migrate app to status
johannesvedder Jun 11, 2024
1cb7740
fix: do not show separator if no delete item shown
johannesvedder Jun 11, 2024
b6dad92
fix: only show close button for editors
johannesvedder Jun 11, 2024
547c468
fix: add closed success description
johannesvedder Jun 11, 2024
4beb796
style: format
johannesvedder Jun 11, 2024
9ffc7ce
tests: migrate db test to status
johannesvedder Jun 11, 2024
7c5b8ae
tests: fix workflow trigger
johannesvedder Jun 11, 2024
f190d1f
Merge branch 'dev' into feat/study-closing-studystatus
johannesvedder Jun 12, 2024
e27a580
chore: format
johannesvedder Jun 12, 2024
dad913e
chore: format
johannesvedder Jun 12, 2024
cdcbca5
style: rephrase translation
johannesvedder Jun 13, 2024
9b66d33
feat: new severe close confirmation
johannesvedder Jun 13, 2024
931c4e2
Merge branch 'feat/study-closing' into feat/study-closing-studystatus
johannesvedder Jun 14, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Prev Previous commit
Next Next commit
feat: deprecate published for study_status
  • Loading branch information
johannesvedder committed Jun 11, 2024
commit 3cf8804463bb10d8e5d8a25aa5cd99664e0a7cee
4 changes: 0 additions & 4 deletions app/lib/screens/study/onboarding/study_selection.dart
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,6 @@ class _StudySelectionScreenState extends State<StudySelectionScreen> {
child: StudyTile.fromStudy(
study: study,
onTap: () async {
if (study.isClosed) {
await showStudyClosedDialog(context);
return;
}
await navigateToStudyOverview(context, study);
},
),
Expand Down
19 changes: 5 additions & 14 deletions core/lib/src/models/tables/study.dart
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ class Study extends SupabaseObjectFunctions<Study> implements Comparable<Study>
late Contact contact = Contact();
@JsonKey(name: 'icon_name', defaultValue: 'accountHeart')
late String iconName = 'accountHeart';
@Deprecated('Use status instead')
@JsonKey(defaultValue: false)
late bool published = false;
@JsonKey(name: 'is_closed', defaultValue: false)
bool isClosed = false;
late StudyStatus status = StudyStatus.draft;
@JsonKey(fromJson: _questionnaireFromJson)
late StudyUQuestionnaire questionnaire = StudyUQuestionnaire();
@JsonKey(name: 'eligibility_criteria', fromJson: _eligibilityCriteriaFromJson)
Expand Down Expand Up @@ -231,7 +231,7 @@ class Study extends SupabaseObjectFunctions<Study> implements Comparable<Study>
static Future<ExtractionResult<Study>> publishedPublicStudies() async {
ExtractionResult<Study> result;
try {
final response = await env.client.from(tableName).select().eq('participation', 'open').eq("is_closed", false);
final response = await env.client.from(tableName).select().eq('participation', 'open').eq("closed", false);
final extracted = SupabaseQuery.extractSupabaseList<Study>(List<Map<String, dynamic>>.from(response));
result = ExtractionSuccess<Study>(extracted);
} on ExtractionFailedException<Study> catch (error) {
Expand Down Expand Up @@ -292,26 +292,17 @@ class Study extends SupabaseObjectFunctions<Study> implements Comparable<Study>

// - Status

StudyStatus get status {
if (isClosed) {
return StudyStatus.closed;
}
if (published) {
return StudyStatus.running;
}
return StudyStatus.draft;
}

bool get isDraft => status == StudyStatus.draft;
bool get isRunning => status == StudyStatus.running;
bool get isClosed => status == StudyStatus.closed;

bool isReadonly(User user) {
return status != StudyStatus.draft || !canEdit(user);
}

@override
String toString() {
return 'Study{id: $id, title: $title, description: $description, userId: $userId, participation: $participation, resultSharing: $resultSharing, contact: $contact, iconName: $iconName, published: $published, questionnaire: $questionnaire, eligibilityCriteria: $eligibilityCriteria, consent: $consent, interventions: $interventions, observations: $observations, schedule: $schedule, reportSpecification: $reportSpecification, results: $results, collaboratorEmails: $collaboratorEmails, registryPublished: $registryPublished, participantCount: $participantCount, endedCount: $endedCount, activeSubjectCount: $activeSubjectCount, missedDays: $missedDays, repo: $repo, invites: $invites, participants: $participants, participantsProgress: $participantsProgress, createdAt: $createdAt}';
return 'Study{id: $id, title: $title, description: $description, userId: $userId, participation: $participation, resultSharing: $resultSharing, contact: $contact, iconName: $iconName, published: <deprecated>, status: $status, questionnaire: $questionnaire, eligibilityCriteria: $eligibilityCriteria, consent: $consent, interventions: $interventions, observations: $observations, schedule: $schedule, reportSpecification: $reportSpecification, results: $results, collaboratorEmails: $collaboratorEmails, registryPublished: $registryPublished, participantCount: $participantCount, endedCount: $endedCount, activeSubjectCount: $activeSubjectCount, missedDays: $missedDays, repo: $repo, invites: $invites, participants: $participants, participantsProgress: $participantsProgress, createdAt: $createdAt}';
}

@override
Expand Down
146 changes: 146 additions & 0 deletions database/migration/20240526_migrate_close_study.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
-- todo move to studyu-schema.sql

CREATE TYPE public.study_status AS ENUM (
'draft',
'running',
'closed'
);

ALTER TABLE public.study
ADD COLUMN status public.study_status DEFAULT 'draft'::public.study_status NOT NULL;

-- Migrate existing studies from published to study_status
UPDATE public.study SET status = CASE
WHEN status != 'draft'::public.study_status THEN status
WHEN published THEN 'running'::public.study_status
ELSE status
END;

-- Migrate policy
DROP POLICY "Editors can do everything with their studies" ON public.study;

CREATE POLICY "Editors can view their studies" ON public.study FOR SELECT USING (auth.uid() = user_id);

CREATE POLICY "Editor can control their draft studies" ON public.study
USING (public.can_edit(auth.uid(), study.*) AND status = 'draft'::public.study_status);

-- Editors can only update registry_published and resultSharing
--grant update (registry_published, result_sharing) on public.study USING (public.can_edit(auth.uid(), study.*);
--CREATE POLICY "Editors can only update registry_published and resultSharing" ON public.study
-- FOR UPDATE
-- USING (public.can_edit(auth.uid(), study.*))
-- WITH CHECK ((new.*) IS NOT DISTINCT FROM (old.* EXCEPT registry_published, result_sharing));
-- todo solve with trigger or function
-- or create view with only updatable columns and provide permission on view see https://dba.stackexchange.com/questions/298931/allow-users-to-modify-only-some-but-not-all-fields-in-a-postgresql-table-with

-- https://stackoverflow.com/questions/72756376/supabase-solutions-for-column-level-security

-- https://github.com/orgs/supabase/discussions/656#discussioncomment-5594653
CREATE OR REPLACE FUNCTION public.allow_updating_only()
RETURNS trigger
LANGUAGE plpgsql
AS $function$
DECLARE
whitelist TEXT[] := TG_ARGV::TEXT[];
schema_table TEXT;
column_name TEXT;
rec RECORD;
new_value TEXT;
old_value TEXT;
BEGIN
schema_table := concat(TG_TABLE_SCHEMA, '.', TG_TABLE_NAME);

-- If RLS is not active on current table for function invoker, early return
IF NOT row_security_active(schema_table) THEN
RETURN NEW;
END IF;

-- Otherwise, loop on all columns of the table schema
FOR rec IN (
SELECT col.column_name
FROM information_schema.columns as col
WHERE table_schema = TG_TABLE_SCHEMA
AND table_name = TG_TABLE_NAME
) LOOP
-- If the current column is whitelisted, early continue
column_name := rec.column_name;
IF column_name = ANY(whitelist) THEN
CONTINUE;
END IF;

-- If not whitelisted, execute dynamic SQL to get column value from OLD and NEW records
EXECUTE format('SELECT ($1).%I, ($2).%I', column_name, column_name)
INTO new_value, old_value
USING NEW, OLD;

-- Raise exception if column value changed
IF new_value IS DISTINCT FROM old_value THEN
RAISE EXCEPTION 'Unauthorized change to "%"', column_name;
END IF;
END LOOP;

-- RLS active, but no exception encountered, clear to proceed.
RETURN NEW;
END;
$function$;

CREATE OR REPLACE FUNCTION public.check_study_update_permissions()
RETURNS trigger
LANGUAGE plpgsql
AS $function$
BEGIN
-- todo check if this is needed, because the policy should already prevent modification of foreign studies
--IF study_param.user_id != auth.uid() THEN
-- RAISE EXCEPTION 'Only the owner can update the status';
--END IF;

-- check if old.status is not draft and if yes check if allow_updating_only includes registry_published and result_sharing otherwise raise exception
IF OLD.status != 'draft'::public.study_status THEN
PERFORM public.allow_updating_only(ARRAY['registry_published', 'result_sharing']);
-- dont allow to update status directly
IF NEW.status != OLD.status THEN
RAISE EXCEPTION 'Study.status can only be updated using the `public.update_study_status` function';
END IF;
END IF;

RETURN NEW;
END;
$function$;

CREATE OR REPLACE TRIGGER study_status_update_permissions
BEFORE UPDATE
ON public.study
FOR EACH ROW
EXECUTE FUNCTION public.check_study_update_permissions();

-- todo use this function to update status in designer
-- Owners can update status
CREATE FUNCTION public.update_study_status(study_param public.study) RETURNS VOID
LANGUAGE plpgsql -- SECURITY DEFINER
AS $$
BEGIN
--IF study_param.user_id != auth.uid() THEN
-- RAISE EXCEPTION 'Only the owner can update the status';
--END IF;
-- Increment the study.status
UPDATE public.study
SET status = CASE
WHEN study_param.status = 'draft'::public.study_status THEN 'running'::public.study_status
WHEN study_param.status = 'running'::public.study_status THEN 'closed'::public.study_status
ELSE study_param.status
END
WHERE id = study_param.id;
END;
$$;

ALTER FUNCTION public.update_study_status(public.study) OWNER TO postgres;

CREATE POLICY "Joining a closed study should not be possible" ON public.study_subject
AS RESTRICTIVE
FOR INSERT
WITH CHECK (NOT EXISTS (
SELECT 1
FROM public.study
WHERE study.id = study_subject.study_id
AND study.status = 'closed'::public.study_status
));
12 changes: 0 additions & 12 deletions database/migration/migrate-close_study.sql

This file was deleted.

16 changes: 1 addition & 15 deletions database/studyu-schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ CREATE TABLE public.study (
title text NOT NULL,
description text NOT NULL,
icon_name text NOT NULL,
-- published is deprecated, use status instead
published boolean DEFAULT false NOT NULL,
is_closed boolean DEFAULT false NOT NULL,
registry_published boolean DEFAULT false NOT NULL,
questionnaire jsonb NOT NULL,
eligibility_criteria jsonb NOT NULL,
Expand Down Expand Up @@ -836,20 +836,6 @@ CREATE POLICY "Editors can see subjects from their studies" ON public.study_subj
CREATE POLICY "Invite code needs to be valid (not possible in the app)" ON public.study_subject AS RESTRICTIVE FOR INSERT WITH CHECK (((invite_code IS NULL) OR (study_id IN ( SELECT code_fun.study_id
FROM public.get_study_from_invite(study_subject.invite_code) code_fun(study_id, preselected_intervention_ids)))));

--
-- Name: study_subject Joining a closed study should not be possible; Type: POLICY; Schema: public; Owner: postgres
--

CREATE POLICY "Joining a closed study should not be possible" ON public.study_subject
AS RESTRICTIVE
FOR INSERT
WITH CHECK (NOT EXISTS (
SELECT 1
FROM public.study
WHERE study.id = study_subject.study_id
AND study.is_closed
));

--
-- Name: subject_progress Editors can see their study subjects progress; Type: POLICY; Schema: public; Owner: postgres
--
Expand Down
5 changes: 2 additions & 3 deletions designer_v2/lib/domain/study.dart
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,7 @@ extension StudyDuplicateX on Study {
copy.resetJsonIgnoredAttributes();
copy.title = (copy.title ?? '').withDuplicateLabel();
copy.userId = userId;
copy.published = false;
copy.isClosed = false;
copy.status = StudyStatus.draft;
copy.resultSharing = ResultSharing.private;
copy.registryPublished = false;
copy.results = [];
Expand All @@ -141,7 +140,7 @@ extension StudyDuplicateX on Study {
final copy = Study.fromJson(toJson());
copy.copyJsonIgnoredAttributes(from: this, createdAt: true);
copy.resetParticipantData();
copy.published = true;
copy.status = StudyStatus.running;
return copy;
}

Expand Down
3 changes: 1 addition & 2 deletions designer_v2/lib/features/recruit/study_recruit_page.dart
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class StudyRecruitScreen extends StudyPageWidget {
@override
Widget? banner(BuildContext context, WidgetRef ref) {
final state = ref.watch(studyRecruitControllerProvider(studyId));
final isStudyClosed = state.studyWithMetadata?.model.isClosed == true;
final isStudyClosed = state.studyWithMetadata!.model.isClosed;

if (isStudyClosed) {
return BannerBox(
Expand All @@ -74,7 +74,6 @@ class StudyRecruitScreen extends StudyPageWidget {
]),
style: BannerStyle.info);
}

return null;
}

Expand Down
2 changes: 1 addition & 1 deletion designer_v2/lib/features/study/study_controller_state.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class StudyControllerState extends StudyControllerBaseState implements IStudyApp
this.lastSynced,
});

bool get isPublished => study.value != null && study.value!.published;
bool get isPublished => study.value != null && study.value!.status == StudyStatus.running;

// - ISyncIndicatorViewModel

Expand Down
2 changes: 1 addition & 1 deletion designer_v2/lib/repositories/model_repository.dart
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ abstract class ModelRepository<T> extends IModelRepository<T> {
if (fetchOnSubscribe) {
if (!(wrappedModel != null && wrappedModel.isLocalOnly)) {
fetch(modelId).catchError((e) {
if (!modelController.isClosed) {
if (!modelController.closed) {
modelController.addError(e);
}
return e;
Expand Down
2 changes: 1 addition & 1 deletion designer_v2/lib/repositories/study_repository.dart
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class StudyRepository extends ModelRepository<Study> implements IStudyRepository
}

Future<void> onCloseCallback() {
model.isClosed = true;
model.status = StudyStatus.closed;
return save(model).then((value) => ref.read(routerProvider).dispatch(RoutingIntents.studies)).then((value) =>
Future.delayed(const Duration(milliseconds: 200),
() => ref.read(notificationServiceProvider).show(Notifications.studyClosed)));
Expand Down