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 all commits
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/supabase-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ name: Test Supabase
on:
push:
paths:
- ".github/workflows/supabase-test.yml"
- "database/**"
- "supabase/**"
- ".github/workflows/supabase-test.yml"
workflow_dispatch:

jobs:
Expand Down
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 @@ -172,10 +172,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 @@ -55,10 +55,10 @@ class Study extends SupabaseObjectFunctions<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 @@ -245,7 +245,7 @@ class Study extends SupabaseObjectFunctions<Study>
.from(tableName)
.select()
.eq('participation', 'open')
.eq("is_closed", false);
.neq('status', StudyStatus.closed.name);
final extracted = SupabaseQuery.extractSupabaseList<Study>(
List<Map<String, dynamic>>.from(response),
);
Expand Down Expand Up @@ -319,26 +319,17 @@ class Study extends SupabaseObjectFunctions<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
10 changes: 8 additions & 2 deletions core/lib/src/models/tables/study.g.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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

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

ALTER TYPE public.study_status OWNER TO postgres;

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;

DROP POLICY "Everybody can view designated published studies" ON public.study;

CREATE POLICY "Study visibility" ON public.study FOR SELECT
USING ((status = 'running'::public.study_status OR status = 'closed'::public.study_status)
AND (registry_published = true OR participation = 'open'::public.participation OR result_sharing = 'public'::public.result_sharing));
-- todo should we allow draft studies in registry if they have been published?

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
-- old --USING (public.can_edit(auth.uid(), study.*) AND status = 'draft'::public.study_status);
-- USING (public.can_edit(auth.uid(), study.*));

-- 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));
-- t odo 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_study()
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

-- The user 'postgres' should be able to update any record, e.g. when using Supabase Studio
IF CURRENT_USER = 'postgres' THEN
RETURN NEW;
END IF;

-- In draft status allow update of all columns
IF OLD.status = 'draft'::public.study_status THEN
RETURN NEW;
END IF;

-- Only allow status to be updated from draft to running and from running to closed
IF OLD.status != NEW.status THEN
IF NOT (
(OLD.status = 'draft'::public.study_status AND NEW.status = 'running'::public.study_status)
OR (OLD.status = 'running'::public.study_status AND NEW.status = 'closed'::public.study_status)
) THEN
RAISE EXCEPTION 'Invalid status transition';
END IF;
END IF;

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$;

ALTER FUNCTION public.allow_updating_only_study() OWNER TO postgres;

-- Only allow updating status, registry_published and result_sharing of the study table when in draft mode
CREATE OR REPLACE TRIGGER study_status_update_permissions
BEFORE UPDATE
ON public.study
FOR EACH ROW
EXECUTE FUNCTION public.allow_updating_only_study('updated_at', 'status', 'registry_published', 'result_sharing');
-- todo also add participation?

-- 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.

Loading
Loading