From 6396024435574b2248500780bde613c8b63c33e9 Mon Sep 17 00:00:00 2001 From: Luuc van der Zee Date: Mon, 8 Jul 2024 14:37:03 +0100 Subject: [PATCH 01/17] Add new volunteering permissions action --- back/app/models/permission.rb | 2 +- back/app/services/permissions/base_permissions_service.rb | 1 + .../services/permissions/project_permissions_service.rb | 8 +++++++- back/spec/acceptance/projects_spec.rb | 3 ++- 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/back/app/models/permission.rb b/back/app/models/permission.rb index 235d8fec9294..e769857955e9 100644 --- a/back/app/models/permission.rb +++ b/back/app/models/permission.rb @@ -29,7 +29,7 @@ class Permission < ApplicationRecord 'survey' => %w[taking_survey attending_event], 'poll' => %w[taking_poll attending_event], 'voting' => %w[voting commenting_idea attending_event], - 'volunteering' => %w[attending_event], + 'volunteering' => %w[volunteering attending_event], 'document_annotation' => %w[annotating_document attending_event] } SCOPE_TYPES = [nil, 'Phase'].freeze diff --git a/back/app/services/permissions/base_permissions_service.rb b/back/app/services/permissions/base_permissions_service.rb index b8998f0ce582..9f541b61473e 100644 --- a/back/app/services/permissions/base_permissions_service.rb +++ b/back/app/services/permissions/base_permissions_service.rb @@ -14,6 +14,7 @@ class BasePermissionsService taking_survey taking_poll attending_event + volunteering ].freeze USER_DENIED_REASONS = { diff --git a/back/app/services/permissions/project_permissions_service.rb b/back/app/services/permissions/project_permissions_service.rb index 838f27796b8d..0b511bae9cee 100644 --- a/back/app/services/permissions/project_permissions_service.rb +++ b/back/app/services/permissions/project_permissions_service.rb @@ -12,7 +12,7 @@ def denied_reason_for_action(action, user, project, reaction_mode: nil) project_visible_reason else phase = @timeline_service.current_phase_not_archived project - super action, user, phase, project: project, reaction_mode: reaction_mode + super(action, user, phase, project: project, reaction_mode: reaction_mode) end end @@ -33,6 +33,8 @@ def action_descriptors(project, user) taking_poll_disabled_reason = denied_reason_for_action 'taking_poll', user, project voting_disabled_reason = denied_reason_for_action 'voting', user, project attending_event_disabled_reason = denied_reason_for_action 'attending_event', user, project + volunteering_disabled_reason = denied_reason_for_action 'volunteering', user, project + { posting_idea: { enabled: !posting_disabled_reason, @@ -79,6 +81,10 @@ def action_descriptors(project, user) attending_event: { enabled: !attending_event_disabled_reason, disabled_reason: attending_event_disabled_reason + }, + volunteering: { + enabled: !volunteering_disabled_reason, + disabled_reason: volunteering_disabled_reason } } end diff --git a/back/spec/acceptance/projects_spec.rb b/back/spec/acceptance/projects_spec.rb index 3ce414b66637..1a52e7bea10a 100644 --- a/back/spec/acceptance/projects_spec.rb +++ b/back/spec/acceptance/projects_spec.rb @@ -174,7 +174,8 @@ taking_survey: { enabled: false, disabled_reason: 'project_inactive' }, taking_poll: { enabled: false, disabled_reason: 'project_inactive' }, voting: { enabled: false, disabled_reason: 'project_inactive' }, - attending_event: { enabled: false, disabled_reason: 'project_inactive' } + attending_event: { enabled: false, disabled_reason: 'project_inactive' }, + volunteering: { enabled: false, disabled_reason: 'project_inactive' } } ) expect(json_response.dig(:data, :relationships)).to include( From 63b4c14598bbb73e59d1c17f6b99ad192ed1950e Mon Sep 17 00:00:00 2001 From: Luuc van der Zee Date: Tue, 9 Jul 2024 11:52:03 +0100 Subject: [PATCH 02/17] Create failing test for volunteers spec --- .../spec/acceptance/volunteers_spec.rb | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/back/engines/free/volunteering/spec/acceptance/volunteers_spec.rb b/back/engines/free/volunteering/spec/acceptance/volunteers_spec.rb index f94f3060a929..d44a2471b649 100644 --- a/back/engines/free/volunteering/spec/acceptance/volunteers_spec.rb +++ b/back/engines/free/volunteering/spec/acceptance/volunteers_spec.rb @@ -31,6 +31,41 @@ do_request assert_status 422 end + + context 'when the phase has granular permissions' do + let(:user) { create(:user) } + let(:group) { create(:group) } + + let(:project) do + create( + :single_phase_volunteering_project, + phase_attrs: { with_permissions: true } + ) + end + + let(:cause) do + cause = create(:cause, phase: project.phases.first) + permission = cause.phase.permissions.find_by(action: 'volunteering') + permission.update!(permitted_by: 'groups', groups: [group]) + + cause + end + + let(:cause_id) { cause.id } + + before { header_token_for(user) } + + example 'Try to volunteering for a cause, not as a group member', document: false do + do_request + assert_status 401 + end + + example 'Try to volunteering for a cause, as a group member', document: false do + group.add_member(user).save! + do_request + assert_status 201 + end + end end delete 'web_api/v1/causes/:cause_id/volunteers' do From e2cb86c13c207d792fbcf933cc70c01a95d7f9a8 Mon Sep 17 00:00:00 2001 From: Luuc van der Zee Date: Tue, 9 Jul 2024 13:11:16 +0100 Subject: [PATCH 03/17] Make sure volunteering phases in acceptance tests are active --- .../app/policies/volunteering/volunteer_policy.rb | 10 +++++++--- .../engines/free/volunteering/spec/factories/causes.rb | 8 +++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/back/engines/free/volunteering/app/policies/volunteering/volunteer_policy.rb b/back/engines/free/volunteering/app/policies/volunteering/volunteer_policy.rb index e904ec20350f..c9c4bd338564 100644 --- a/back/engines/free/volunteering/app/policies/volunteering/volunteer_policy.rb +++ b/back/engines/free/volunteering/app/policies/volunteering/volunteer_policy.rb @@ -25,9 +25,13 @@ def index_xlsx? end def create? - user&.active? && - (record.user_id == user.id) && - ProjectPolicy.new(user, record.cause.phase.project).show? + return false unless user&.active? + return false unless record.user_id == user.id + + reason = Permissions::ProjectPermissionsService.new.denied_reason_for_action 'volunteering', user, record.cause.phase.project + return false if reason + + ProjectPolicy.new(user, record.cause.phase.project).show? end def destroy? diff --git a/back/engines/free/volunteering/spec/factories/causes.rb b/back/engines/free/volunteering/spec/factories/causes.rb index 70ca1e85e041..fb64270e8a06 100644 --- a/back/engines/free/volunteering/spec/factories/causes.rb +++ b/back/engines/free/volunteering/spec/factories/causes.rb @@ -2,7 +2,13 @@ FactoryBot.define do factory :cause, class: 'Volunteering::Cause' do - phase { create(:volunteering_phase) } + phase do + create( + :volunteering_phase, + start_at: Faker::Date.between(from: 6.months.ago, to: Time.zone.now), + end_at: nil + ) + end sequence(:title_multiloc) do |n| { 'en' => "Good cause #{n}", From 31e92e4d9836a78ea6ab18d5a77c268fe6d498e5 Mon Sep 17 00:00:00 2001 From: Luuc van der Zee Date: Tue, 9 Jul 2024 13:27:35 +0100 Subject: [PATCH 04/17] _ --- .../spec/acceptance/volunteers_spec.rb | 24 +++++++++++++++++-- .../volunteering/spec/factories/causes.rb | 15 ++++++------ 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/back/engines/free/volunteering/spec/acceptance/volunteers_spec.rb b/back/engines/free/volunteering/spec/acceptance/volunteers_spec.rb index d44a2471b649..7d311a801d58 100644 --- a/back/engines/free/volunteering/spec/acceptance/volunteers_spec.rb +++ b/back/engines/free/volunteering/spec/acceptance/volunteers_spec.rb @@ -16,7 +16,17 @@ post 'web_api/v1/causes/:cause_id/volunteers' do ValidationErrorHelper.new.error_fields(self, Volunteering::Volunteer) - let(:cause) { create(:cause) } + let(:cause) do + create( + :cause, + phase: create( + :volunteering_phase, + start_at: 6.months.ago, + end_at: nil + ) + ) + end + let(:cause_id) { cause.id } example_request 'Create a volunteer with the current user' do @@ -69,7 +79,17 @@ end delete 'web_api/v1/causes/:cause_id/volunteers' do - let(:cause) { create(:cause) } + let(:cause) do + create( + :cause, + phase: create( + :volunteering_phase, + start_at: 6.months.ago, + end_at: nil + ) + ) + end + let(:cause_id) { cause.id } let!(:volunteer) { create(:volunteer, user: @user, cause: cause) } diff --git a/back/engines/free/volunteering/spec/factories/causes.rb b/back/engines/free/volunteering/spec/factories/causes.rb index fb64270e8a06..c46a7d75776a 100644 --- a/back/engines/free/volunteering/spec/factories/causes.rb +++ b/back/engines/free/volunteering/spec/factories/causes.rb @@ -2,13 +2,14 @@ FactoryBot.define do factory :cause, class: 'Volunteering::Cause' do - phase do - create( - :volunteering_phase, - start_at: Faker::Date.between(from: 6.months.ago, to: Time.zone.now), - end_at: nil - ) - end + # phase do + # create( + # :volunteering_phase, + # start_at: Faker::Date.between(from: 6.months.ago, to: Time.zone.now), + # end_at: nil + # ) + # end + phase { create(:volunteering_phase) } sequence(:title_multiloc) do |n| { 'en' => "Good cause #{n}", From 7bb15c90cd145e09d5442c5b76f4852896d7794f Mon Sep 17 00:00:00 2001 From: Luuc van der Zee Date: Tue, 9 Jul 2024 16:55:02 +0100 Subject: [PATCH 05/17] _ --- back/app/services/permissions/base_permissions_service.rb | 2 +- .../spec/services/report_builder/queries/projects_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/back/app/services/permissions/base_permissions_service.rb b/back/app/services/permissions/base_permissions_service.rb index 9f541b61473e..0316385f946c 100644 --- a/back/app/services/permissions/base_permissions_service.rb +++ b/back/app/services/permissions/base_permissions_service.rb @@ -91,7 +91,7 @@ def user_requirements_service end def user_can_moderate_something?(user) - @user_can_moderate_something ||= (user.admin? || UserRoleService.new.moderates_something?(user)) + @user_can_moderate_something ||= user.admin? || UserRoleService.new.moderates_something?(user) end end end diff --git a/back/engines/commercial/report_builder/spec/services/report_builder/queries/projects_spec.rb b/back/engines/commercial/report_builder/spec/services/report_builder/queries/projects_spec.rb index 0a9af3ab7dc4..5c68c1004c38 100644 --- a/back/engines/commercial/report_builder/spec/services/report_builder/queries/projects_spec.rb +++ b/back/engines/commercial/report_builder/spec/services/report_builder/queries/projects_spec.rb @@ -8,7 +8,7 @@ describe '#run_query' do before_all do # 2020 - past_project = create(:project) + past_project = create(:single_phase_ideation_project) create(:phase, project: past_project, start_at: Date.new(2020, 2, 1), end_at: Date.new(2020, 3, 1)) # 2021 From d6bc913c5445ab154e5c3f7c4a7dd4167b3c9fb2 Mon Sep 17 00:00:00 2001 From: Luuc van der Zee Date: Tue, 9 Jul 2024 19:25:16 +0100 Subject: [PATCH 06/17] Hopefully fix all tests now --- .../permissions/phase_permissions_service.rb | 14 ++++++++++++- .../report_builder/queries/projects_spec.rb | 20 ++++++++++++------- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/back/app/services/permissions/phase_permissions_service.rb b/back/app/services/permissions/phase_permissions_service.rb index 0a28c115221c..554cb60839a8 100644 --- a/back/app/services/permissions/phase_permissions_service.rb +++ b/back/app/services/permissions/phase_permissions_service.rb @@ -42,6 +42,10 @@ class PhasePermissionsService < BasePermissionsService already_responded: 'already_responded' }.freeze + VOLUNTEERING_DENIED_REASONS = { + not_volunteering: 'not_volunteering' + }.freeze + def denied_reason_for_action(action, user, phase, project: phase&.project, reaction_mode: nil) return PHASE_DENIED_REASONS[:project_inactive] unless phase @@ -60,12 +64,14 @@ def denied_reason_for_action(action, user, phase, project: phase&.project, react taking_survey_denied_reason_for_phase(phase) when 'taking_poll' taking_poll_denied_reason_for_phase(phase, user) + when 'volunteering' + volunteering_denied_reason_for_phase(phase, user) else raise "Unsupported action: #{action}" unless SUPPORTED_ACTIONS.include?(action) end return phase_denied_reason if phase_denied_reason - super action, user, phase, project: project + super(action, user, phase, project: project) end alias denied_reason_for_phase denied_reason_for_action @@ -131,6 +137,12 @@ def voting_denied_reason_for_phase(phase, _user) end end + def volunteering_denied_reason_for_phase(phase, _user) + unless phase.volunteering? + VOLUNTEERING_DENIED_REASONS[:not_volunteering] + end + end + # Helper methods def posting_limit_reached?(phase, user) diff --git a/back/engines/commercial/report_builder/spec/services/report_builder/queries/projects_spec.rb b/back/engines/commercial/report_builder/spec/services/report_builder/queries/projects_spec.rb index 5c68c1004c38..6f86064a328b 100644 --- a/back/engines/commercial/report_builder/spec/services/report_builder/queries/projects_spec.rb +++ b/back/engines/commercial/report_builder/spec/services/report_builder/queries/projects_spec.rb @@ -8,25 +8,31 @@ describe '#run_query' do before_all do # 2020 - past_project = create(:single_phase_ideation_project) - create(:phase, project: past_project, start_at: Date.new(2020, 2, 1), end_at: Date.new(2020, 3, 1)) + past_project = create(:project) + create( + :phase, + project: past_project, + start_at: Date.new(2020, 2, 1), + end_at: Date.new(2020, 3, 1), + with_permissions: true + ) # 2021 @project1 = create(:project) - create(:phase, project: @project1, start_at: Date.new(2021, 2, 1), end_at: Date.new(2021, 3, 1)) + create(:phase, project: @project1, start_at: Date.new(2021, 2, 1), end_at: Date.new(2021, 3, 1), with_permissions: true) @project2 = create(:project) - create(:phase, project: @project2, start_at: Date.new(2021, 2, 1), end_at: Date.new(2021, 3, 1)) - create(:phase, project: @project2, start_at: Date.new(2021, 3, 2), end_at: nil) + create(:phase, project: @project2, start_at: Date.new(2021, 2, 1), end_at: Date.new(2021, 3, 1), with_permissions: true) + create(:phase, project: @project2, start_at: Date.new(2021, 3, 2), end_at: nil, with_permissions: true) # 2022 @project3 = create(:project) - create(:phase, project: @project3, start_at: Date.new(2022, 2, 1), end_at: nil) + create(:phase, project: @project3, start_at: Date.new(2022, 2, 1), end_at: nil, with_permissions: true) # Project not published (should be filtered out) project4 = create(:project) project4.admin_publication.update!(publication_status: 'draft') - create(:phase, project: project4, start_at: Date.new(2022, 2, 1), end_at: nil) + create(:phase, project: project4, start_at: Date.new(2022, 2, 1), end_at: nil, with_permissions: true) # Empty project create(:project) From 3c46202fc97c89916bf039bdff0c2db4451742fb Mon Sep 17 00:00:00 2001 From: Luuc van der Zee Date: Tue, 9 Jul 2024 19:37:41 +0100 Subject: [PATCH 07/17] Remove comment --- back/engines/free/volunteering/spec/factories/causes.rb | 7 ------- 1 file changed, 7 deletions(-) diff --git a/back/engines/free/volunteering/spec/factories/causes.rb b/back/engines/free/volunteering/spec/factories/causes.rb index c46a7d75776a..70ca1e85e041 100644 --- a/back/engines/free/volunteering/spec/factories/causes.rb +++ b/back/engines/free/volunteering/spec/factories/causes.rb @@ -2,13 +2,6 @@ FactoryBot.define do factory :cause, class: 'Volunteering::Cause' do - # phase do - # create( - # :volunteering_phase, - # start_at: Faker::Date.between(from: 6.months.ago, to: Time.zone.now), - # end_at: nil - # ) - # end phase { create(:volunteering_phase) } sequence(:title_multiloc) do |n| { From 3f3b4e1363fd91f4ed695f54fb1d00924ef30101 Mon Sep 17 00:00:00 2001 From: Luuc van der Zee Date: Tue, 9 Jul 2024 21:08:46 +0100 Subject: [PATCH 08/17] _ --- back/db/structure.sql | 34 +++++++++---------- front/app/api/permissions/types.ts | 3 +- .../containers/Granular/messages.ts | 4 +++ .../containers/Granular/utils.tsx | 1 + .../Admin/projects/project/phase/messages.ts | 4 +++ .../Admin/projects/project/phase/utils.ts | 4 ++- 6 files changed, 31 insertions(+), 19 deletions(-) diff --git a/back/db/structure.sql b/back/db/structure.sql index 71880e31a039..e397507cff55 100644 --- a/back/db/structure.sql +++ b/back/db/structure.sql @@ -1107,10 +1107,10 @@ CREATE TABLE public.official_feedbacks ( -- CREATE VIEW public.analytics_build_feedbacks AS - SELECT a.post_id, - min(a.feedback_first_date) AS feedback_first_date, - max(a.feedback_official) AS feedback_official, - max(a.feedback_status_change) AS feedback_status_change + SELECT post_id, + min(feedback_first_date) AS feedback_first_date, + max(feedback_official) AS feedback_official, + max(feedback_status_change) AS feedback_status_change FROM ( SELECT activities.item_id AS post_id, min(activities.created_at) AS feedback_first_date, 0 AS feedback_official, @@ -1125,7 +1125,7 @@ CREATE VIEW public.analytics_build_feedbacks AS 0 AS feedback_status_change FROM public.official_feedbacks GROUP BY official_feedbacks.post_id) a - GROUP BY a.post_id; + GROUP BY post_id; -- @@ -1190,8 +1190,8 @@ CREATE TABLE public.projects ( -- CREATE VIEW public.analytics_dimension_projects AS - SELECT projects.id, - projects.title_multiloc + SELECT id, + title_multiloc FROM public.projects; @@ -1431,11 +1431,11 @@ CREATE TABLE public.events ( -- CREATE VIEW public.analytics_fact_events AS - SELECT events.id, - events.project_id AS dimension_project_id, - (events.created_at)::date AS dimension_date_created_id, - (events.start_at)::date AS dimension_date_start_id, - (events.end_at)::date AS dimension_date_end_id + SELECT id, + project_id AS dimension_project_id, + (created_at)::date AS dimension_date_created_id, + (start_at)::date AS dimension_date_start_id, + (end_at)::date AS dimension_date_end_id FROM public.events; @@ -1979,11 +1979,11 @@ CREATE TABLE public.impact_tracking_sessions ( -- CREATE VIEW public.analytics_fact_sessions AS - SELECT impact_tracking_sessions.id, - impact_tracking_sessions.monthly_user_hash, - (impact_tracking_sessions.created_at)::date AS dimension_date_created_id, - (impact_tracking_sessions.updated_at)::date AS dimension_date_updated_id, - impact_tracking_sessions.user_id AS dimension_user_id + SELECT id, + monthly_user_hash, + (created_at)::date AS dimension_date_created_id, + (updated_at)::date AS dimension_date_updated_id, + user_id AS dimension_user_id FROM public.impact_tracking_sessions; diff --git a/front/app/api/permissions/types.ts b/front/app/api/permissions/types.ts index 2bc2f9aab675..564e8f60d03f 100644 --- a/front/app/api/permissions/types.ts +++ b/front/app/api/permissions/types.ts @@ -40,7 +40,8 @@ export type IPhasePermissionAction = | 'taking_poll' | 'voting' | 'annotating_document' - | 'attending_event'; + | 'attending_event' + | 'volunteering'; export interface IPhasePermissionData { id: string; diff --git a/front/app/containers/Admin/projects/project/permissions/granular_permissions/containers/Granular/messages.ts b/front/app/containers/Admin/projects/project/permissions/granular_permissions/containers/Granular/messages.ts index 0043111173e8..b5319d212980 100644 --- a/front/app/containers/Admin/projects/project/permissions/granular_permissions/containers/Granular/messages.ts +++ b/front/app/containers/Admin/projects/project/permissions/granular_permissions/containers/Granular/messages.ts @@ -76,6 +76,10 @@ export default defineMessages({ id: 'app.containers.AdminPage.groups.permissions.permissionAction_attending_event_subtitle', defaultMessage: 'Who can sign up to attend an event?', }, + permissionAction_volunteering_subtitle: { + id: 'app.containers.AdminPage.groups.permissions.permissionAction_volunteering_subtitle', + defaultMessage: 'Who can volunteer?', + }, phase: { id: 'app.containers.AdminPage.groups.permissions.phase', defaultMessage: 'Phase ', diff --git a/front/app/containers/Admin/projects/project/permissions/granular_permissions/containers/Granular/utils.tsx b/front/app/containers/Admin/projects/project/permissions/granular_permissions/containers/Granular/utils.tsx index 5e401ad90ffd..a307e80478ba 100644 --- a/front/app/containers/Admin/projects/project/permissions/granular_permissions/containers/Granular/utils.tsx +++ b/front/app/containers/Admin/projects/project/permissions/granular_permissions/containers/Granular/utils.tsx @@ -43,6 +43,7 @@ export const getPermissionActionSectionSubtitle = ({ annotating_document: messages.permissionAction_annotating_document_subtitle, attending_event: messages.permissionAction_attending_event_subtitle, + volunteering: messages.permissionAction_volunteering_subtitle, }; return participationContextPermissionActionMessages[permissionAction]; } diff --git a/front/app/containers/Admin/projects/project/phase/messages.ts b/front/app/containers/Admin/projects/project/phase/messages.ts index da5c64709446..bae9141b2542 100644 --- a/front/app/containers/Admin/projects/project/phase/messages.ts +++ b/front/app/containers/Admin/projects/project/phase/messages.ts @@ -123,6 +123,10 @@ export default defineMessages({ id: 'app.components.app.containers.AdminPage.ProjectEdit.phaseHeader.attendingEvent', defaultMessage: 'Attending event: {participants}', }, + volunteering: { + id: 'app.components.app.containers.AdminPage.ProjectEdit.phaseHeader.volunteering', + defaultMessage: 'Volunteering: {participants}', + }, and: { id: 'app.components.app.containers.AdminPage.ProjectEdit.phaseHeader.and', defaultMessage: 'and', diff --git a/front/app/containers/Admin/projects/project/phase/utils.ts b/front/app/containers/Admin/projects/project/phase/utils.ts index d0fb4b575274..e82ec0c2a84c 100644 --- a/front/app/containers/Admin/projects/project/phase/utils.ts +++ b/front/app/containers/Admin/projects/project/phase/utils.ts @@ -76,7 +76,9 @@ export const getParticipationActionLabel = (action: IPhasePermissionAction) => { case 'annotating_document': return messages.annotatingDocument; case 'attending_event': - return messages.annotatingDocument; + return messages.attendingEvent; + case 'volunteering': + return messages.volunteering; } }; From 64c5d72115407011c53907f495eabc522e7b2b61 Mon Sep 17 00:00:00 2001 From: Luuc van der Zee Date: Wed, 10 Jul 2024 10:27:06 +0100 Subject: [PATCH 09/17] Trigger auth flow with correct context on click volunteer button --- .../shared/volunteering/CauseCard.tsx | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/front/app/containers/ProjectsShowPage/shared/volunteering/CauseCard.tsx b/front/app/containers/ProjectsShowPage/shared/volunteering/CauseCard.tsx index fb3a37f3d21e..a5c3dd72f42c 100644 --- a/front/app/containers/ProjectsShowPage/shared/volunteering/CauseCard.tsx +++ b/front/app/containers/ProjectsShowPage/shared/volunteering/CauseCard.tsx @@ -13,8 +13,8 @@ import { } from '@citizenlab/cl2-component-library'; import styled, { useTheme } from 'styled-components'; -import { GLOBAL_CONTEXT } from 'api/authentication/authentication_requirements/constants'; import getAuthenticationRequirements from 'api/authentication/authentication_requirements/getAuthenticationRequirements'; +import { AuthenticationContext } from 'api/authentication/authentication_requirements/types'; import { ICauseData } from 'api/causes/types'; import useAddVolunteer from 'api/causes/useAddVolunteer'; import useDeleteVolunteer from 'api/causes/useDeleteVolunteer'; @@ -192,13 +192,24 @@ const CauseCard = ({ cause, className, disabled }: Props) => { } as const; const handleOnVolunteerButtonClick = async () => { - const response = await getAuthenticationRequirements(GLOBAL_CONTEXT); + const phaseId = cause.relationships.phase.data.id; + + const context: AuthenticationContext = { + type: 'phase', + action: 'volunteering', + id: phaseId, + }; + + const response = await getAuthenticationRequirements(context); const { requirements } = response.data.attributes; if (requirements.permitted) { volunteer(); } else { - triggerAuthenticationFlow({ successAction }); + triggerAuthenticationFlow({ + successAction, + context, + }); } }; From d49abd6c5ff3a4c577f7e91fc33b69c4df344bf4 Mon Sep 17 00:00:00 2001 From: Luuc van der Zee Date: Wed, 10 Jul 2024 10:40:45 +0100 Subject: [PATCH 10/17] Use action descriptor to control behavior volunteering button instead of custom logic --- .../shared/volunteering/CauseCard.tsx | 25 +++++++++++-------- .../shared/volunteering/index.tsx | 21 ++-------------- 2 files changed, 17 insertions(+), 29 deletions(-) diff --git a/front/app/containers/ProjectsShowPage/shared/volunteering/CauseCard.tsx b/front/app/containers/ProjectsShowPage/shared/volunteering/CauseCard.tsx index a5c3dd72f42c..956f9e821008 100644 --- a/front/app/containers/ProjectsShowPage/shared/volunteering/CauseCard.tsx +++ b/front/app/containers/ProjectsShowPage/shared/volunteering/CauseCard.tsx @@ -13,11 +13,11 @@ import { } from '@citizenlab/cl2-component-library'; import styled, { useTheme } from 'styled-components'; -import getAuthenticationRequirements from 'api/authentication/authentication_requirements/getAuthenticationRequirements'; import { AuthenticationContext } from 'api/authentication/authentication_requirements/types'; import { ICauseData } from 'api/causes/types'; import useAddVolunteer from 'api/causes/useAddVolunteer'; import useDeleteVolunteer from 'api/causes/useDeleteVolunteer'; +import { IProject } from 'api/projects/types'; import { triggerAuthenticationFlow } from 'containers/Authentication/events'; @@ -27,6 +27,7 @@ import Image from 'components/UI/Image'; import QuillEditedContent from 'components/UI/QuillEditedContent'; import { ScreenReaderOnly } from 'utils/a11y'; +import { isFixableByAuthentication } from 'utils/actionDescriptors'; import { FormattedMessage, useIntl } from 'utils/cl-intl'; import { isEmptyMultiloc } from 'utils/helperUtils'; @@ -165,10 +166,10 @@ const ActionWrapper = styled.div` interface Props { cause: ICauseData; className?: string; - disabled?: boolean; + project: IProject; } -const CauseCard = ({ cause, className, disabled }: Props) => { +const CauseCard = ({ cause, className, project }: Props) => { const { mutate: addVolunteer } = useAddVolunteer(); const { mutate: deleteVolunteer } = useDeleteVolunteer(); const theme = useTheme(); @@ -191,6 +192,13 @@ const CauseCard = ({ cause, className, disabled }: Props) => { params: { cause }, } as const; + const { disabled_reason } = + project.data.attributes.action_descriptors.volunteering; + + const blocked = !!disabled_reason; + const blockedAndUnfixable = + blocked && !isFixableByAuthentication(disabled_reason); + const handleOnVolunteerButtonClick = async () => { const phaseId = cause.relationships.phase.data.id; @@ -200,12 +208,9 @@ const CauseCard = ({ cause, className, disabled }: Props) => { id: phaseId, }; - const response = await getAuthenticationRequirements(context); - const { requirements } = response.data.attributes; - - if (requirements.permitted) { + if (!blocked) { volunteer(); - } else { + } else if (!blockedAndUnfixable) { triggerAuthenticationFlow({ successAction, context, @@ -270,7 +275,7 @@ const CauseCard = ({ cause, className, disabled }: Props) => { @@ -280,7 +285,7 @@ const CauseCard = ({ cause, className, disabled }: Props) => { icon={!isVolunteer ? 'volunteer' : 'volunteer-off'} buttonStyle={!isVolunteer ? 'primary' : 'secondary-outlined'} fullWidth={smallerThanSmallTablet} - disabled={disabled} + disabled={blockedAndUnfixable} > {isVolunteer ? ( diff --git a/front/app/containers/ProjectsShowPage/shared/volunteering/index.tsx b/front/app/containers/ProjectsShowPage/shared/volunteering/index.tsx index cf7edfc6791a..6f111ed3d840 100644 --- a/front/app/containers/ProjectsShowPage/shared/volunteering/index.tsx +++ b/front/app/containers/ProjectsShowPage/shared/volunteering/index.tsx @@ -3,11 +3,8 @@ import React, { memo } from 'react'; import styled from 'styled-components'; import useCauses from 'api/causes/useCauses'; -import usePhase from 'api/phases/usePhase'; import useProjectById from 'api/projects/useProjectById'; -import { pastPresentOrFuture } from 'utils/dateUtils'; - import CauseCard from './CauseCard'; const Container = styled.div` @@ -25,26 +22,12 @@ const Volunteering = memo(({ projectId, phaseId, className }) => { phaseId, }); const { data: project } = useProjectById(projectId); - const { data: phase } = usePhase(phaseId); - - if (causes && phase) { - const disabledPhase = - phase && - pastPresentOrFuture([ - phase.data.attributes.start_at, - phase.data.attributes.end_at, - ]) !== 'present'; - const disabledProject = - project && project.data.attributes.publication_status !== 'published'; + if (causes && project) { return ( {causes.data.map((cause) => ( - + ))} ); From cf88a07883d5ac11ef1ae4292094b9ea8810a885 Mon Sep 17 00:00:00 2001 From: Luuc van der Zee Date: Wed, 10 Jul 2024 11:00:19 +0100 Subject: [PATCH 11/17] Revert changes to DB structure --- back/db/structure.sql | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/back/db/structure.sql b/back/db/structure.sql index e397507cff55..71880e31a039 100644 --- a/back/db/structure.sql +++ b/back/db/structure.sql @@ -1107,10 +1107,10 @@ CREATE TABLE public.official_feedbacks ( -- CREATE VIEW public.analytics_build_feedbacks AS - SELECT post_id, - min(feedback_first_date) AS feedback_first_date, - max(feedback_official) AS feedback_official, - max(feedback_status_change) AS feedback_status_change + SELECT a.post_id, + min(a.feedback_first_date) AS feedback_first_date, + max(a.feedback_official) AS feedback_official, + max(a.feedback_status_change) AS feedback_status_change FROM ( SELECT activities.item_id AS post_id, min(activities.created_at) AS feedback_first_date, 0 AS feedback_official, @@ -1125,7 +1125,7 @@ CREATE VIEW public.analytics_build_feedbacks AS 0 AS feedback_status_change FROM public.official_feedbacks GROUP BY official_feedbacks.post_id) a - GROUP BY post_id; + GROUP BY a.post_id; -- @@ -1190,8 +1190,8 @@ CREATE TABLE public.projects ( -- CREATE VIEW public.analytics_dimension_projects AS - SELECT id, - title_multiloc + SELECT projects.id, + projects.title_multiloc FROM public.projects; @@ -1431,11 +1431,11 @@ CREATE TABLE public.events ( -- CREATE VIEW public.analytics_fact_events AS - SELECT id, - project_id AS dimension_project_id, - (created_at)::date AS dimension_date_created_id, - (start_at)::date AS dimension_date_start_id, - (end_at)::date AS dimension_date_end_id + SELECT events.id, + events.project_id AS dimension_project_id, + (events.created_at)::date AS dimension_date_created_id, + (events.start_at)::date AS dimension_date_start_id, + (events.end_at)::date AS dimension_date_end_id FROM public.events; @@ -1979,11 +1979,11 @@ CREATE TABLE public.impact_tracking_sessions ( -- CREATE VIEW public.analytics_fact_sessions AS - SELECT id, - monthly_user_hash, - (created_at)::date AS dimension_date_created_id, - (updated_at)::date AS dimension_date_updated_id, - user_id AS dimension_user_id + SELECT impact_tracking_sessions.id, + impact_tracking_sessions.monthly_user_hash, + (impact_tracking_sessions.created_at)::date AS dimension_date_created_id, + (impact_tracking_sessions.updated_at)::date AS dimension_date_updated_id, + impact_tracking_sessions.user_id AS dimension_user_id FROM public.impact_tracking_sessions; From c31298ebbf278cc0446c0ebcccca7f3ceaf17d3c Mon Sep 17 00:00:00 2001 From: Luuc van der Zee Date: Wed, 17 Jul 2024 08:40:06 +0100 Subject: [PATCH 12/17] Remove redundant user creation in spec --- .../services/report_builder/queries/projects_spec.rb | 10 +++++----- .../volunteering/spec/acceptance/volunteers_spec.rb | 5 +---- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/back/engines/commercial/report_builder/spec/services/report_builder/queries/projects_spec.rb b/back/engines/commercial/report_builder/spec/services/report_builder/queries/projects_spec.rb index 6f86064a328b..d50b6d8df144 100644 --- a/back/engines/commercial/report_builder/spec/services/report_builder/queries/projects_spec.rb +++ b/back/engines/commercial/report_builder/spec/services/report_builder/queries/projects_spec.rb @@ -19,20 +19,20 @@ # 2021 @project1 = create(:project) - create(:phase, project: @project1, start_at: Date.new(2021, 2, 1), end_at: Date.new(2021, 3, 1), with_permissions: true) + create(:phase, project: @project1, start_at: Date.new(2021, 2, 1), end_at: Date.new(2021, 3, 1)) @project2 = create(:project) - create(:phase, project: @project2, start_at: Date.new(2021, 2, 1), end_at: Date.new(2021, 3, 1), with_permissions: true) - create(:phase, project: @project2, start_at: Date.new(2021, 3, 2), end_at: nil, with_permissions: true) + create(:phase, project: @project2, start_at: Date.new(2021, 2, 1), end_at: Date.new(2021, 3, 1)) + create(:phase, project: @project2, start_at: Date.new(2021, 3, 2), end_at: nil) # 2022 @project3 = create(:project) - create(:phase, project: @project3, start_at: Date.new(2022, 2, 1), end_at: nil, with_permissions: true) + create(:phase, project: @project3, start_at: Date.new(2022, 2, 1), end_at: nil) # Project not published (should be filtered out) project4 = create(:project) project4.admin_publication.update!(publication_status: 'draft') - create(:phase, project: project4, start_at: Date.new(2022, 2, 1), end_at: nil, with_permissions: true) + create(:phase, project: project4, start_at: Date.new(2022, 2, 1), end_at: nil) # Empty project create(:project) diff --git a/back/engines/free/volunteering/spec/acceptance/volunteers_spec.rb b/back/engines/free/volunteering/spec/acceptance/volunteers_spec.rb index 7d311a801d58..6695ed3ecfb8 100644 --- a/back/engines/free/volunteering/spec/acceptance/volunteers_spec.rb +++ b/back/engines/free/volunteering/spec/acceptance/volunteers_spec.rb @@ -43,7 +43,6 @@ end context 'when the phase has granular permissions' do - let(:user) { create(:user) } let(:group) { create(:group) } let(:project) do @@ -63,15 +62,13 @@ let(:cause_id) { cause.id } - before { header_token_for(user) } - example 'Try to volunteering for a cause, not as a group member', document: false do do_request assert_status 401 end example 'Try to volunteering for a cause, as a group member', document: false do - group.add_member(user).save! + group.add_member(@user).save! do_request assert_status 201 end From 98612d065b46739102210bd6e52c71e2759e8a9d Mon Sep 17 00:00:00 2001 From: Luuc van der Zee Date: Wed, 17 Jul 2024 08:40:46 +0100 Subject: [PATCH 13/17] Fix example descriptions --- .../free/volunteering/spec/acceptance/volunteers_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/back/engines/free/volunteering/spec/acceptance/volunteers_spec.rb b/back/engines/free/volunteering/spec/acceptance/volunteers_spec.rb index 6695ed3ecfb8..805c5e110a9a 100644 --- a/back/engines/free/volunteering/spec/acceptance/volunteers_spec.rb +++ b/back/engines/free/volunteering/spec/acceptance/volunteers_spec.rb @@ -62,12 +62,12 @@ let(:cause_id) { cause.id } - example 'Try to volunteering for a cause, not as a group member', document: false do + example 'Try to volunteer for a cause, not as a group member', document: false do do_request assert_status 401 end - example 'Try to volunteering for a cause, as a group member', document: false do + example 'Try to volunteer for a cause, as a group member', document: false do group.add_member(@user).save! do_request assert_status 201 From 4606f9b3b0e3497f0ef54fa68496f39b90a6b5b1 Mon Sep 17 00:00:00 2001 From: Luuc van der Zee Date: Wed, 17 Jul 2024 09:59:20 +0100 Subject: [PATCH 14/17] Add volunteering disabled reason messages --- front/app/utils/actionDescriptors/index.ts | 9 +++++++ front/app/utils/actionDescriptors/messages.ts | 26 +++++++++++++++++++ front/app/utils/actionDescriptors/types.ts | 4 ++- 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/front/app/utils/actionDescriptors/index.ts b/front/app/utils/actionDescriptors/index.ts index db95b32db7ae..7cb9ebac1902 100644 --- a/front/app/utils/actionDescriptors/index.ts +++ b/front/app/utils/actionDescriptors/index.ts @@ -120,6 +120,15 @@ const actionDisabledMessages: { user_not_verified: messages.attendingEventNotVerified, user_missing_requirements: messages.attendingEventMissingRequirements, }, + volunteering: { + user_not_signed_in: messages.volunteeringNotSignedIn, + user_not_active: messages.volunteeringNotActiveUser, + user_not_verified: messages.volunteeringNotVerified, + user_missing_requirements: messages.volunteeringMissingRequirements, + user_not_permitted: messages.volunteeringNotPermitted, + user_not_in_group: messages.volunteeringNotInGroup, + user_blocked: messages.volunteeringNotPermitted, + }, }; /** diff --git a/front/app/utils/actionDescriptors/messages.ts b/front/app/utils/actionDescriptors/messages.ts index 23e306621a22..877fa004412c 100644 --- a/front/app/utils/actionDescriptors/messages.ts +++ b/front/app/utils/actionDescriptors/messages.ts @@ -256,4 +256,30 @@ export default defineMessages({ id: 'app.utils.actionDescriptors.attendingEventMissingRequirements', defaultMessage: 'You must complete your profile to attend this event.', }, + + // volunteering + volunteeringNotSignedIn: { + id: 'app.utils.actionDescriptors.volunteeringNotSignedIn', + defaultMessage: 'You must log in or register to volunteer.', + }, + volunteeringNotActiveUser: { + id: 'app.utils.actionDescriptors.volunteeringdNotActiveUser', + defaultMessage: 'Please {completeRegistrationLink} to volunteer.', + }, + volunteeringNotVerified: { + id: 'app.utils.actionDescriptors.volunteeringNotVerified', + defaultMessage: 'You must verify your account before you can volunteer.', + }, + volunteeringMissingRequirements: { + id: 'app.utils.actionDescriptors.volunteeringMissingRequirements', + defaultMessage: 'You must complete your profile to volunteer.', + }, + volunteeringNotPermitted: { + id: 'app.utils.actionDescriptors.volunteeringNotPermitted', + defaultMessage: 'You are not permitted to volunteer.', + }, + volunteeringNotInGroup: { + id: 'app.utils.actionDescriptors.volunteeringNotInGroup', + defaultMessage: 'You do not meet the requirements to volunteer.', + }, }); diff --git a/front/app/utils/actionDescriptors/types.ts b/front/app/utils/actionDescriptors/types.ts index afb14b7abc3f..179c7db10588 100644 --- a/front/app/utils/actionDescriptors/types.ts +++ b/front/app/utils/actionDescriptors/types.ts @@ -103,7 +103,8 @@ export type ActionDescriptorAction = | 'annotating_document' | 'taking_survey' | 'taking_poll' - | 'attending_event'; + | 'attending_event' + | 'volunteering'; // All disabled reasons export type DisabledReason = @@ -115,6 +116,7 @@ export type DisabledReason = | ProjectPollDisabledReason | ProjectDocumentAnnotationDisabledReason | ProjectVotingDisabledReason + | ProjectVolunteeringDisabledReason | IdeaReactingDisabledReason | IdeaCommentingDisabledReason | IdeaVotingDisabledReason; From 58737745b988d7f2e7f876b80d1b783f6c3f66d0 Mon Sep 17 00:00:00 2001 From: Luuc van der Zee Date: Wed, 17 Jul 2024 10:17:04 +0100 Subject: [PATCH 15/17] Add custom message in volunteering tooltip --- .../ProjectsShowPage/shared/volunteering/CauseCard.tsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/front/app/containers/ProjectsShowPage/shared/volunteering/CauseCard.tsx b/front/app/containers/ProjectsShowPage/shared/volunteering/CauseCard.tsx index 956f9e821008..c91845001761 100644 --- a/front/app/containers/ProjectsShowPage/shared/volunteering/CauseCard.tsx +++ b/front/app/containers/ProjectsShowPage/shared/volunteering/CauseCard.tsx @@ -27,7 +27,10 @@ import Image from 'components/UI/Image'; import QuillEditedContent from 'components/UI/QuillEditedContent'; import { ScreenReaderOnly } from 'utils/a11y'; -import { isFixableByAuthentication } from 'utils/actionDescriptors'; +import { + isFixableByAuthentication, + getPermissionsDisabledMessage, +} from 'utils/actionDescriptors'; import { FormattedMessage, useIntl } from 'utils/cl-intl'; import { isEmptyMultiloc } from 'utils/helperUtils'; @@ -277,7 +280,10 @@ const CauseCard = ({ cause, className, project }: Props) => {