From c97ccb68c8f48a17b258cad725a332114c1b9ecd Mon Sep 17 00:00:00 2001 From: Jasper Mayone Date: Fri, 9 Oct 2026 12:27:35 -0400 Subject: [PATCH 1/3] feat: ask legacy slack members to accept the code of conduct --- app/controllers/home_controller.rb | 8 +- .../slack_onboarding_controller.rb | 26 ++++- app/controllers/webhooks/slack_controller.rb | 75 +++++++++++--- app/jobs/code_of_conduct_request_job.rb | 56 +++++++++++ .../slack_code_of_conduct_accepted_job.rb | 29 +++++- app/mailers/user_mailer.rb | 15 +++ app/models/user.rb | 20 +++- app/serializers/user_serializer.rb | 1 + app/services/code_of_conduct_acceptance.rb | 57 +++++++++++ app/services/slack_service.rb | 97 ++++++++++++++++--- app/services/slack_webhook_service.rb | 2 +- app/views/admin/users/show.html.erb | 5 + app/views/slack_onboarding/show.html.erb | 34 +++++-- .../code_of_conduct_request.html.erb | 27 ++++++ .../code_of_conduct_request.text.erb | 17 ++++ app/views/users/show.html.erb | 11 ++- config/locales/code_of_conduct_request.en.yml | 19 ++++ ...000_add_slack_coc_requested_at_to_users.rb | 9 ++ db/schema.rb | 4 +- lib/tasks/slack.rake | 47 +++++++++ spec/factories/users.rb | 1 + spec/jobs/code_of_conduct_request_job_spec.rb | 60 ++++++++++++ ...slack_code_of_conduct_accepted_job_spec.rb | 16 +++ spec/mailers/previews/user_mailer_preview.rb | 6 ++ spec/mailers/user_mailer_spec.rb | 11 +++ spec/models/user_slack_membership_spec.rb | 44 ++++++++- spec/models/user_spec.rb | 1 + spec/requests/slack_interactions_spec.rb | 68 +++++++++++++ spec/requests/slack_onboarding_spec.rb | 67 ++++++++++++- spec/serializers/user_serializer_spec.rb | 1 + spec/services/slack_webhook_service_spec.rb | 2 +- test/fixtures/users.yml | 1 + test/models/user_test.rb | 1 + 33 files changed, 785 insertions(+), 53 deletions(-) create mode 100644 app/jobs/code_of_conduct_request_job.rb create mode 100644 app/services/code_of_conduct_acceptance.rb create mode 100644 app/views/user_mailer/code_of_conduct_request.html.erb create mode 100644 app/views/user_mailer/code_of_conduct_request.text.erb create mode 100644 config/locales/code_of_conduct_request.en.yml create mode 100644 db/migrate/20261009120000_add_slack_coc_requested_at_to_users.rb create mode 100644 spec/jobs/code_of_conduct_request_job_spec.rb diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb index a7cb7ce..ecacb05 100644 --- a/app/controllers/home_controller.rb +++ b/app/controllers/home_controller.rb @@ -8,11 +8,13 @@ class HomeController < ApplicationController def index # Signed-in members land on their account dashboard, not the # "you found the front door" splash (which is for anonymous visitors). - # Joining the Slack is part of becoming a member, so anyone who hasn't - # finished lands on the next step of that instead. + # Joining the Slack (which includes accepting the code of conduct) is part + # of becoming a member, so anyone who hasn't finished lands on the next + # step of that instead. return unless current_user - redirect_to(current_user.email_verified? && current_user.slack_pending? ? slack_onboarding_path : profile_path) + onboarding = current_user.email_verified? && !current_user.slack_onboarding_complete? + redirect_to(onboarding ? slack_onboarding_path : profile_path) end end diff --git a/app/controllers/slack_onboarding_controller.rb b/app/controllers/slack_onboarding_controller.rb index b114c79..55e4da2 100644 --- a/app/controllers/slack_onboarding_controller.rb +++ b/app/controllers/slack_onboarding_controller.rb @@ -42,16 +42,34 @@ def create end # Accepts the code of conduct from the web, for people who can't find the - # Slack DM. Also retries the promotion if it failed after an earlier - # acceptance. + # Slack DM and for full members who joined before the code-of-conduct flow. + # Also asks for a real name when the Slack import could not find one, and + # retries the promotion if it failed after an earlier acceptance. def accept_code_of_conduct unless %i[accept_code_of_conduct awaiting_promotion].include?(current_user.slack_onboarding_step) redirect_to slack_onboarding_path return end - SlackCodeOfConductAcceptedJob.perform_later(current_user.slack_id) - redirect_to slack_onboarding_path, notice: "Thanks for accepting the Code of Conduct. Your full Slack access is on its way." + result = CodeOfConductAcceptance.call( + current_user, + first_name: params[:first_name], + last_name: params[:last_name] + ) + + unless result.success? + @step = current_user.slack_onboarding_step + @name_errors = result.errors + render :show, status: :unprocessable_content + return + end + + notice = if current_user.slack_member? + "Thanks for accepting the Code of Conduct." + else + "Thanks for accepting the Code of Conduct. Your full Slack access is on its way." + end + redirect_to slack_onboarding_path, notice: notice end end diff --git a/app/controllers/webhooks/slack_controller.rb b/app/controllers/webhooks/slack_controller.rb index ccb8e3e..4d4adf5 100644 --- a/app/controllers/webhooks/slack_controller.rb +++ b/app/controllers/webhooks/slack_controller.rb @@ -57,26 +57,41 @@ def events render json: { status: "ok" }, status: :ok end - # Slack interactivity (Block Kit button clicks). Handles the "I accept the - # Code of Conduct" button, which promotes a single-channel guest to a full - # member. Slack posts the interaction as a form-encoded `payload` field. + # Slack interactivity. Handles the code-of-conduct DM (see + # SlackService#post_code_of_conduct): + # + # - "accept_coc" button: accepts at once, and promotes a single-channel + # guest to a full member. + # - "open_coc_form" button: opens the form that asks for a missing name. + # - Submission of that form: saves the name and accepts. + # + # Slack posts the interaction as a form-encoded `payload` field. def interactions payload = JSON.parse(params[:payload].to_s.presence || "{}") + + if payload["type"] == "view_submission" + return submit_code_of_conduct_form(payload) if payload.dig("view", "callback_id") == SlackService::CODE_OF_CONDUCT_FORM + + head :ok + return + end + action = (payload["actions"] || []).first || {} - if action["action_id"] == "accept_coc" + case action["action_id"] + when "accept_coc" slack_user_id = action["value"].presence || payload.dig("user", "id") SlackCodeOfConductAcceptedJob.perform_later(slack_user_id) if slack_user_id.present? # Replace the original message so the button can't be used twice. - render json: { - replace_original: true, - text: ":white_check_mark: Thanks for accepting the Code of Conduct โ€” you now have full access to the Patchwork Labs Slack. Welcome! :tada:" - } - return + already_member = User.find_by(slack_id: slack_user_id)&.slack_member? || false + render json: { replace_original: true, text: SlackService.code_of_conduct_thanks(already_member:) } + when "open_coc_form" + open_code_of_conduct_form(payload) + head :ok + else + head :ok end - - head :ok rescue JSON::ParserError => e Rails.logger.error "[Slack Interactions] Invalid payload: #{e.message}" head :bad_request @@ -84,6 +99,44 @@ def interactions private + # Called while Slack waits for the answer to the click: the trigger_id + # expires after 3 seconds. + def open_code_of_conduct_form(payload) + SlackService.new.open_code_of_conduct_form( + trigger_id: payload["trigger_id"], + user: User.find_by(slack_id: payload.dig("user", "id")), + message: { channel: payload.dig("container", "channel_id"), ts: payload.dig("container", "message_ts") }.compact.presence + ) + rescue => e + Rails.logger.error "[Slack Interactions] Could not open CoC form: #{e.message}" + end + + # The person who submits the form is the one who accepts, so the user + # comes from the payload's user, never from a value in the form. + def submit_code_of_conduct_form(payload) + slack_user_id = payload.dig("user", "id") + values = payload.dig("view", "state", "values") || {} + message = JSON.parse(payload.dig("view", "private_metadata").presence || "{}").symbolize_keys.slice(:channel, :ts).presence + user = User.find_by(slack_id: slack_user_id) + + if user + result = CodeOfConductAcceptance.call( + user, + first_name: values.dig("first_name", "value", "value"), + last_name: values.dig("last_name", "value", "value"), + message: message + ) + unless result.success? + render json: { response_action: "errors", errors: result.errors.transform_keys(&:to_s) } + return + end + elsif slack_user_id.present? + SlackCodeOfConductAcceptedJob.perform_later(slack_user_id, **{ message: message }.compact) + end + + render json: { response_action: "clear" } + end + def devise_configured? defined?(Devise) end diff --git a/app/jobs/code_of_conduct_request_job.rb b/app/jobs/code_of_conduct_request_job.rb new file mode 100644 index 0000000..8df1751 --- /dev/null +++ b/app/jobs/code_of_conduct_request_job.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +# Asks a full Slack member who joined before the code-of-conduct flow to +# accept it (User.code_of_conduct_pending). Sends the request two ways, +# because many members don't check Slack often: +# +# - a Slack DM with a button that accepts (or opens a form that also asks for +# a missing name), and +# - an email with a link to sign in to Weave and accept on /slack. +# +# The text is in config/locales/code_of_conduct_request.en.yml. Enqueued by +# `bin/rails slack:coc:request`. Idempotent: a member who has accepted is +# skipped, and so is one who was already asked, unless reminder is true. +class CodeOfConductRequestJob < ApplicationJob + queue_as :default + + # deadline is the date after which members who have not accepted move to + # guest access. nil leaves the deadline out of the message. + def perform(user_id, deadline: nil, reminder: false) + user = User.find_by(id: user_id) + return unless user&.can_authenticate? + return if user.slack_coc_accepted_at.present? + return if user.slack_coc_requested_at.present? && !reminder + + send_slack_dm(user, deadline) if user.slack_id.present? + UserMailer.code_of_conduct_request(user, deadline: deadline).deliver_later + user.update_columns(slack_coc_requested_at: Time.current) # rubocop:disable Rails/SkipsModelValidations + end + + # The opening of the message, as plain-text paragraphs. Shared by the Slack + # DM and the email. + def self.paragraphs(user, deadline: nil) + t = ->(key, **args) { I18n.t("code_of_conduct_request.#{key}", **args) } + [ + # Capitalized like User#full_name: many Slack-imported names are lowercase. + t.call(:greeting, first_name: user.name_missing? ? "there" : user.first_name.gsub(/\b\p{L}/, &:upcase)), + *t.call(:update).split(/\n{2,}/), + (t.call(:name_missing) if user.name_missing?), + (t.call(:deadline, deadline: deadline.strftime("%B %-d, %Y")) if deadline) + ].compact + end + + private + + # A Slack failure must not stop the email. + def send_slack_dm(user, deadline) + service = SlackService.new + return unless service.configured? + + intro = [*self.class.paragraphs(user, deadline:), I18n.t("code_of_conduct_request.slack_action")].join("\n\n") + service.post_code_of_conduct(user.slack_id, intro: intro, collect_name: user.name_missing?) + rescue => e + Rails.logger.error "Failed to DM the CoC request to #{user.slack_id}: #{e.message}" + end + +end diff --git a/app/jobs/slack_code_of_conduct_accepted_job.rb b/app/jobs/slack_code_of_conduct_accepted_job.rb index f7ade37..4286e1e 100644 --- a/app/jobs/slack_code_of_conduct_accepted_job.rb +++ b/app/jobs/slack_code_of_conduct_accepted_job.rb @@ -1,13 +1,20 @@ # frozen_string_literal: true -# Runs when a single-channel guest accepts the code of conduct, either with the -# button in their Slack DM or on the /slack onboarding page. Records the -# acceptance, promotes them from guest to full workspace member, and marks -# them a member in Weave once Slack confirms the promotion. +# Runs when someone accepts the code of conduct: with the button in their Slack +# DM, with the Slack form, or on the /slack onboarding page. Records the +# acceptance, promotes a single-channel guest to full workspace member, and +# marks them a member in Weave once Slack confirms the promotion. +# +# A full member who joined before the code-of-conduct flow is already a member, +# so only the acceptance is recorded for them. +# +# message is the Slack DM that asked for the acceptance, as { channel:, ts: }. +# When it is given, the DM is replaced with a thank-you so its button can't be +# used again. class SlackCodeOfConductAcceptedJob < ApplicationJob queue_as :default - def perform(slack_user_id) + def perform(slack_user_id, message: nil) return if slack_user_id.blank? user = User.find_by(slack_id: slack_user_id) @@ -16,6 +23,10 @@ def perform(slack_user_id) service = SlackService.new return unless service.configured? + already_member = user&.slack_member? + replace_message(service, message, already_member:) if message.present? + return if already_member + result = service.promote_to_member(slack_user_id) if result[:ok] user&.update_columns(slack_membership: "member", updated_at: Time.current) @@ -27,4 +38,12 @@ def perform(slack_user_id) Rails.logger.error "Slack not configured for promotion: #{e.message}" end + private + + def replace_message(service, message, already_member:) + service.mark_code_of_conduct_accepted(channel: message[:channel], ts: message[:ts], already_member:) + rescue => e + Rails.logger.error "Failed to update CoC DM #{message.inspect}: #{e.message}" + end + end diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 2fa917f..bd98bdf 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -34,4 +34,19 @@ def slack_email_address_added(email_address) ) end + # Asks a member who joined the Slack before the code-of-conduct flow to + # accept it (see CodeOfConductRequestJob). Signing in sends them to /slack, + # which shows the accept step. + def code_of_conduct_request(user, deadline: nil) + @user = user + @paragraphs = CodeOfConductRequestJob.paragraphs(user, deadline:) + @login_url = login_url + @code_of_conduct_url = SlackService.code_of_conduct_url + + mail( + to: @user.email, + subject: env_subject(I18n.t("code_of_conduct_request.subject")) + ) + end + end diff --git a/app/models/user.rb b/app/models/user.rb index a649427..315065a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -28,6 +28,7 @@ # slack_birthday :date # slack_city :string # slack_coc_accepted_at :datetime +# slack_coc_requested_at :datetime # slack_cost_center :string # slack_country :string # slack_department :string @@ -187,6 +188,15 @@ def self.generate_secure_password # as the `slack_member` claim. enum :slack_membership, { pending: "pending", member: "member" }, prefix: :slack + # Full Slack members who have not accepted the code of conduct. They joined + # before the code-of-conduct flow, so nothing has asked them yet (see + # CodeOfConductRequestJob). + scope :code_of_conduct_pending, -> { slack_member.where(slack_coc_accepted_at: nil).where.not(slack_id: nil) } + + # Names the Slack import saves when a Slack profile has no name. + PLACEHOLDER_NAMES = %w[NOTSET Unknown User].freeze + scope :name_missing, -> { where(first_name: PLACEHOLDER_NAMES).or(where(last_name: PLACEHOLDER_NAMES)) } + scope :last_seen_within, ->(ago) { joins(:user_sessions).where(user_sessions: { last_seen_at: ago.. }).distinct } scope :currently_online, -> { last_seen_within(15.minutes.ago) } scope :active, -> { last_seen_within(30.days.ago) } @@ -253,6 +263,9 @@ def self.p_id_format = /\A#{Weave.p_id_prefix}\d[A-F0-9]{9}\z/ end end + # True when the name is one the Slack import made up (see PLACEHOLDER_NAMES). + def name_missing? = PLACEHOLDER_NAMES.include?(first_name) || PLACEHOLDER_NAMES.include?(last_name) + def full_name # Display names capitalized (many Slack-imported names arrive lowercase). # Only upcase the first letter of each word so intentional casing like @@ -472,8 +485,11 @@ def in_slack_workspace? = slack_id.present? # Where this person is in joining the Slack. Drives the /slack onboarding # page. Each step is the next thing the person must do. + # + # Full members who joined before the code-of-conduct flow still have to + # accept it. They are not demoted for that, so they skip :awaiting_promotion. def slack_onboarding_step - return :member if slack_member? + return slack_coc_accepted_at.present? ? :member : :accept_code_of_conduct if slack_member? return :awaiting_promotion if in_slack_workspace? && slack_coc_accepted_at.present? return :accept_code_of_conduct if in_slack_workspace? return :accept_invite if slack_invited_at.present? @@ -481,6 +497,8 @@ def slack_onboarding_step :request_invite end + def slack_onboarding_complete? = slack_onboarding_step == :member + # The membership a Slack user object (from users.info, users.lookupByEmail, # users.list or a team_join/user_change event) entitles its owner to. Guests # and deactivated accounts are not members. diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb index 9c2513e..bcef621 100644 --- a/app/serializers/user_serializer.rb +++ b/app/serializers/user_serializer.rb @@ -28,6 +28,7 @@ # slack_birthday :date # slack_city :string # slack_coc_accepted_at :datetime +# slack_coc_requested_at :datetime # slack_cost_center :string # slack_country :string # slack_department :string diff --git a/app/services/code_of_conduct_acceptance.rb b/app/services/code_of_conduct_acceptance.rb new file mode 100644 index 0000000..815c859 --- /dev/null +++ b/app/services/code_of_conduct_acceptance.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +# Records that a person accepted the code of conduct, from the /slack page or +# the Slack form (see Webhooks::SlackController#interactions). +# +# When the Slack import gave the person a placeholder name (User#name_missing?), +# they must also give their real name. The acceptance is recorded here, at +# once, so the next page they see is up to date. SlackCodeOfConductAcceptedJob +# then does the Slack side: it promotes a guest to full member, and it updates +# the Slack DM when one is given. +class CodeOfConductAcceptance + Result = Data.define(:user, :errors) do + def success? = errors.empty? + end + + # first_name / last_name are read only when the user's name is missing. + # message is the Slack DM to update, as { channel:, ts: }. + def self.call(user, first_name: nil, last_name: nil, message: nil) + new(user, first_name:, last_name:, message:).call + end + + def initialize(user, first_name:, last_name:, message:) + @user = user + @first_name = first_name.to_s.strip + @last_name = last_name.to_s.strip + @message = message + end + + def call + if @user.name_missing? + errors = name_errors + return Result.new(user: @user, errors:) if errors.any? + + @user.update!(first_name: @first_name, last_name: @last_name) + end + + @user.update_columns(slack_coc_accepted_at: Time.current) if @user.slack_coc_accepted_at.nil? # rubocop:disable Rails/SkipsModelValidations + SlackCodeOfConductAcceptedJob.perform_later(@user.slack_id, **{ message: @message }.compact) if @user.slack_id.present? + + Result.new(user: @user, errors: {}) + end + + private + + # Keyed by field, so the Slack form can show each error next to its input. + def name_errors + errors = {} + errors[:first_name] = "Enter your first name." if real_name_part(@first_name).nil? + errors[:last_name] = "Enter your last name." if real_name_part(@last_name).nil? + errors + end + + def real_name_part(value) + value.presence unless User::PLACEHOLDER_NAMES.include?(value) + end + +end diff --git a/app/services/slack_service.rb b/app/services/slack_service.rb index ffc2556..4b4d042 100644 --- a/app/services/slack_service.rb +++ b/app/services/slack_service.rb @@ -39,6 +39,9 @@ class ApiError < SlackError; end MANAGER_FIELD = "Manager" PWL_ID_FIELD = "pwl_idp_id" + # callback_id of the code-of-conduct form (see #open_code_of_conduct_form). + CODE_OF_CONDUCT_FORM = "coc_form" + # How long a workspace's label -> field ID map is cached. CUSTOM_FIELD_IDS_TTL = 1.hour @@ -151,31 +154,44 @@ def promote_to_member(slack_user_id) { ok: false, error: "request_failed", raw: nil } end - # DM a newly-joined guest the code of conduct with an "I accept" button. Uses - # the bot token (needs chat:write + im:write). The button's action_id is - # "accept_coc" and its value is the Slack user id, handled by the interactions - # webhook. Returns the Slack API response. - def post_code_of_conduct(slack_user_id, coc_url: nil) + # DM someone the code of conduct with a button to accept it. Uses the bot + # token (needs chat:write + im:write). Returns the Slack API response. + # + # intro is the opening text (mrkdwn). It defaults to the welcome for a + # newly-joined guest. CodeOfConductRequestJob passes its own. + # + # With collect_name: false, the button accepts at once: its action_id is + # "accept_coc" and its value is the Slack user id. With collect_name: true + # (for people whose name the Slack import could not find), the button + # ("open_coc_form") opens a form that asks for their name and then accepts + # (see #open_code_of_conduct_form). The interactions webhook handles both. + def post_code_of_conduct(slack_user_id, coc_url: nil, intro: nil, collect_name: false) raise ConfigurationError, "Slack client not configured" unless @client coc_url ||= self.class.code_of_conduct_url - intro = "Welcome to Patchwork Labs! :wave: Before you get full access to the community, " \ - "please read our Code of Conduct and accept it below." + intro ||= "*Welcome to Patchwork Labs! :wave: Before you get full access to the community, " \ + "please read our Code of Conduct and accept it below.*" coc_line = coc_url.present? ? "Read it here: #{coc_url}" : "Please review our Code of Conduct." + button = if collect_name + { text: "Review and accept", action_id: "open_coc_form" } + else + { text: "I accept the Code of Conduct", action_id: "accept_coc" } + end + @client.chat_postMessage( channel: slack_user_id, - text: "Please review and accept the Patchwork Labs Code of Conduct to get full access.", + text: "Please review and accept the Patchwork Labs Code of Conduct.", blocks: [ - { type: "section", text: { type: "mrkdwn", text: "*#{intro}*\n\n#{coc_line}" } }, + { type: "section", text: { type: "mrkdwn", text: "#{intro}\n\n#{coc_line}" } }, { type: "actions", elements: [ { type: "button", style: "primary", - text: { type: "plain_text", text: "I accept the Code of Conduct", emoji: true }, - action_id: "accept_coc", + text: { type: "plain_text", text: button[:text], emoji: true }, + action_id: button[:action_id], value: slack_user_id } ] @@ -184,6 +200,56 @@ def post_code_of_conduct(slack_user_id, coc_url: nil) ) end + # Opens the code-of-conduct form from a button click. trigger_id comes from + # the click and expires after 3 seconds, so call this while answering it. + # + # The form asks for a first and last name when the user's name is missing + # (or when there is no linked user, since nothing is known about them). + # message is the DM the button was in, as { channel:, ts: }. It goes into + # private_metadata, so the submission can replace the DM. + def open_code_of_conduct_form(trigger_id:, user:, message: nil) + raise ConfigurationError, "Slack client not configured" unless @client + + coc_url = self.class.code_of_conduct_url + coc_text = coc_url.present? ? "<#{coc_url}|Read the Code of Conduct>" : "Please review our Code of Conduct." + blocks = [ + { type: "section", text: { type: "mrkdwn", text: "#{coc_text}, then choose *I accept* below." } } + ] + if user.nil? || user.name_missing? + blocks.unshift({ type: "section", text: { type: "mrkdwn", text: "We don't have your name yet. Please add it." } }) + blocks.insert(1, name_input("first_name", "First name"), name_input("last_name", "Last name")) + end + + @client.views_open( + trigger_id: trigger_id, + view: { + type: "modal", + callback_id: CODE_OF_CONDUCT_FORM, + private_metadata: (message || {}).to_json, + title: { type: "plain_text", text: "Code of Conduct" }, + submit: { type: "plain_text", text: "I accept" }, + close: { type: "plain_text", text: "Not now" }, + blocks: blocks + } + ) + end + + # Replaces a code-of-conduct DM with a thank-you, so its button can't be + # used again. + def mark_code_of_conduct_accepted(channel:, ts:, already_member: false) + raise ConfigurationError, "Slack client not configured" unless @client + + @client.chat_update(channel: channel, ts: ts, text: self.class.code_of_conduct_thanks(already_member:), blocks: []) + end + + def self.code_of_conduct_thanks(already_member:) + if already_member + ":white_check_mark: Thanks for accepting the Code of Conduct! :tada:" + else + ":white_check_mark: Thanks for accepting the Code of Conduct โ€” you now have full access to the Patchwork Labs Slack. Welcome! :tada:" + end + end + # Where members open the workspace, e.g. https://patchworklabs.slack.com. # nil when the subdomain is not configured. def self.workspace_url @@ -519,6 +585,15 @@ def workspace_subdomain @workspace_subdomain ||= ENV["SLACK_WORKSPACE_SUBDOMAIN"] || Rails.application.credentials.dig(:slack, :workspace_subdomain) end + def name_input(block_id, label) + { + type: "input", + block_id: block_id, + label: { type: "plain_text", text: label }, + element: { type: "plain_text_input", action_id: "value", max_length: 100 } + } + end + # Channel single-channel guests are invited into (the code-of-conduct channel). def coc_channel @coc_channel ||= ENV["SLACK_COC_CHANNEL"] || Rails.application.credentials.dig(:slack, :coc_channel) diff --git a/app/services/slack_webhook_service.rb b/app/services/slack_webhook_service.rb index 14ffc63..054034f 100644 --- a/app/services/slack_webhook_service.rb +++ b/app/services/slack_webhook_service.rb @@ -143,7 +143,7 @@ def send_code_of_conduct(user) return if user&.slack_id.blank? return if user.slack_coc_accepted_at.present? - SlackService.new.post_code_of_conduct(user.slack_id) + SlackService.new.post_code_of_conduct(user.slack_id, collect_name: user.name_missing?) rescue => e Rails.logger.error "[SlackWebhookService] Failed to post CoC to #{user&.slack_id}: #{e.message}" end diff --git a/app/views/admin/users/show.html.erb b/app/views/admin/users/show.html.erb index 8f96808..6a1e7d6 100644 --- a/app/views/admin/users/show.html.erb +++ b/app/views/admin/users/show.html.erb @@ -303,6 +303,11 @@ Full member <% if @user.slack_coc_accepted_at.present? %> accepted CoC <%= @user.slack_coc_accepted_at.strftime("%b %-d, %Y") %> + <% else %> + CoC pending + <% if @user.slack_coc_requested_at.present? %> + asked <%= @user.slack_coc_requested_at.strftime("%b %-d, %Y") %> + <% end %> <% end %> <% elsif @user.in_slack_workspace? && @user.slack_coc_accepted_at.present? %> Guest ยท promotion pending diff --git a/app/views/slack_onboarding/show.html.erb b/app/views/slack_onboarding/show.html.erb index eb5eaa7..a0305f1 100644 --- a/app/views/slack_onboarding/show.html.erb +++ b/app/views/slack_onboarding/show.html.erb @@ -66,18 +66,40 @@ <%= button_to "Resend my invite", slack_onboarding_path, class: "btn-stitch-ghost w-full justify-center" %> <% when :accept_code_of_conduct %> -

- You're in the Slack as a guest. To get full access, accept our Code of - Conduct. We sent it to you as a Slack DM, or you can accept it here. -

+ <% if current_user.slack_member? %> +

+ Everyone in the Patchwork Labs Slack now accepts our Code of Conduct. + Please read it and accept it here. +

+ <% else %> +

+ You're in the Slack as a guest. To get full access, accept our Code of + Conduct. We sent it to you as a Slack DM, or you can accept it here. +

+ <% end %> <% if SlackService.code_of_conduct_url.present? %>

<%= link_to "Read the Code of Conduct", SlackService.code_of_conduct_url, target: "_blank", rel: "noopener noreferrer", class: link_class %>

<% end %> - <%= button_to "I accept the Code of Conduct", accept_code_of_conduct_slack_onboarding_path, - class: "btn-stitch w-full justify-center" %> + <%= form_with url: accept_code_of_conduct_slack_onboarding_path, method: :post, class: "space-y-4 text-left" do |form| %> + <% if current_user.name_missing? %> +

We don't have your name yet. Please add it.

+ <% [[:first_name, "First name"], [:last_name, "Last name"]].each do |field, label| %> +
+ <%= form.label field, label, class: "block text-sm font-medium text-ink" %> + <%= form.text_field field, value: params[field], required: true, maxlength: 100, + autocomplete: field == :first_name ? "given-name" : "family-name", + class: "mt-1 w-full px-4 py-3 rounded-lg border-2 border-neutral-300 bg-paper text-ink focus:ring-2 focus:ring-grape focus:border-grape transition-colors" %> + <% if @name_errors&.key?(field) %> +

<%= @name_errors[field] %>

+ <% end %> +
+ <% end %> + <% end %> + <%= form.submit "I accept the Code of Conduct", class: "btn-stitch w-full justify-center" %> + <% end %> <% when :awaiting_promotion %>

diff --git a/app/views/user_mailer/code_of_conduct_request.html.erb b/app/views/user_mailer/code_of_conduct_request.html.erb new file mode 100644 index 0000000..f47c78f --- /dev/null +++ b/app/views/user_mailer/code_of_conduct_request.html.erb @@ -0,0 +1,27 @@ +

Please accept our Code of Conduct ๐Ÿงต

+ +<% @paragraphs.each do |paragraph| %> +

<%= paragraph %>

+<% end %> + +<% if @code_of_conduct_url.present? %> +

Read the Code of Conduct

+<% end %> + +

<%= t("code_of_conduct_request.email_action", email: @user.email) %>

+ + + + + +
+ + Sign in and accept + +
+ +

<%= t("code_of_conduct_request.sign_off") %>

+ +
+

We also sent this to you as a Slack DM. You only need to accept once.

+
diff --git a/app/views/user_mailer/code_of_conduct_request.text.erb b/app/views/user_mailer/code_of_conduct_request.text.erb new file mode 100644 index 0000000..ef2d386 --- /dev/null +++ b/app/views/user_mailer/code_of_conduct_request.text.erb @@ -0,0 +1,17 @@ +Please accept our Code of Conduct + +<% @paragraphs.each do |paragraph| %> +<%= paragraph %> + +<% end %> +<% if @code_of_conduct_url.present? %> +Read the Code of Conduct: <%= @code_of_conduct_url %> + +<% end %> +<%= t("code_of_conduct_request.email_action", email: @user.email) %> + +<%= @login_url %> + +<%= t("code_of_conduct_request.sign_off") %> + +We also sent this to you as a Slack DM. You only need to accept once. diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 1e82091..f354daf 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -2,11 +2,16 @@
- <% if @user.slack_pending? %> + <% unless @user.slack_onboarding_complete? %>
-

Finish joining the Patchwork Labs Slack

-

You become a full member once you're in the Slack and have accepted the Code of Conduct.

+ <% if @user.slack_member? %> +

Accept the Code of Conduct

+

Everyone in the Patchwork Labs Slack now accepts the Code of Conduct. It takes a few seconds.

+ <% else %> +

Finish joining the Patchwork Labs Slack

+

You become a full member once you're in the Slack and have accepted the Code of Conduct.

+ <% end %>
<%= link_to "Continue", slack_onboarding_path, class: "btn-stitch shrink-0 justify-center" %>
diff --git a/config/locales/code_of_conduct_request.en.yml b/config/locales/code_of_conduct_request.en.yml new file mode 100644 index 0000000..1267a2a --- /dev/null +++ b/config/locales/code_of_conduct_request.en.yml @@ -0,0 +1,19 @@ +# The message that asks members who joined the Slack before the +# code-of-conduct flow to accept it (see CodeOfConductRequestJob). It goes out +# as a Slack DM and as an email, so keep it plain text: no Slack markup and no +# HTML. Blank lines separate paragraphs. +# +# Preview it before sending: bin/rails slack:coc:request +en: + code_of_conduct_request: + subject: "Please accept the Patchwork Labs Code of Conduct" + greeting: "Hi %{first_name}," + update: |- + Here is a quick update on Patchwork Labs. We set up Weave, our new account system. Your Weave account is linked to your Slack account, and it is how you sign in to Patchwork Labs apps. + + New members now join the Slack through Weave, and they accept our Code of Conduct when they join. You joined before we added that step, so we ask you to accept it now. It takes a few seconds. + name_missing: "We also don't have your name yet. Please add it when you accept." + deadline: "Please accept by %{deadline}. After that date, members who have not accepted move to guest access in the Slack until they accept." + slack_action: "Choose the button below to accept." + email_action: "Sign in to Weave with %{email} to accept." + sign_off: "Thanks for being part of Patchwork Labs!" diff --git a/db/migrate/20261009120000_add_slack_coc_requested_at_to_users.rb b/db/migrate/20261009120000_add_slack_coc_requested_at_to_users.rb new file mode 100644 index 0000000..c233345 --- /dev/null +++ b/db/migrate/20261009120000_add_slack_coc_requested_at_to_users.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddSlackCocRequestedAtToUsers < ActiveRecord::Migration[8.1] + def change + # When Weave last asked a member who joined before the code-of-conduct + # flow to accept it (see CodeOfConductRequestJob). + add_column :users, :slack_coc_requested_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index b2090e3..eb76105 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true + # This file is auto-generated from the current state of the database. Instead # of editing this file, please use the migrations feature of Active Record to # incrementally modify your database, and then regenerate this schema definition. @@ -12,7 +13,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_10_07_120000) do +ActiveRecord::Schema[8.1].define(version: 2026_10_09_120000) do # These are extensions that must be enabled in order to support this database enable_extension "fuzzystrmatch" enable_extension "pg_catalog.plpgsql" @@ -588,6 +589,7 @@ t.date "slack_birthday" t.string "slack_city" t.datetime "slack_coc_accepted_at" + t.datetime "slack_coc_requested_at" t.string "slack_cost_center" t.string "slack_country" t.string "slack_department" diff --git a/lib/tasks/slack.rake b/lib/tasks/slack.rake index 2625fd0..0f44331 100644 --- a/lib/tasks/slack.rake +++ b/lib/tasks/slack.rake @@ -67,4 +67,51 @@ namespace :slack do Rake::Task["slack:sync_to_slack"].invoke puts "โœ… Bidirectional sync complete!" end + + # Full members who joined before the code-of-conduct flow never accepted it. + # These tasks ask them to (see CodeOfConductRequestJob). + namespace :coc do + desc "Count full members who have not accepted the code of conduct" + task status: :environment do + pending = User.code_of_conduct_pending + puts "Full members without CoC: #{pending.count}" + puts " asked: #{pending.where.not(slack_coc_requested_at: nil).count}" + puts " not asked yet: #{pending.where(slack_coc_requested_at: nil).count}" + puts " name missing: #{pending.name_missing.count}" + puts "All users with a missing name: #{User.name_missing.count}" + end + + # Shows the message and who gets it. Nothing is sent without SEND=1. + # + # bin/rails slack:coc:request # preview + # bin/rails slack:coc:request SEND=1 # ask everyone not asked yet + # bin/rails slack:coc:request SEND=1 DEADLINE=2026-11-01 + # bin/rails slack:coc:request SEND=1 REMIND=1 # ask again, including people already asked + desc "Ask full members to accept the code of conduct by Slack DM and email (SEND=1 to send)" + task request: :environment do + deadline = ENV["DEADLINE"].presence && Date.iso8601(ENV["DEADLINE"]) + reminder = ENV["REMIND"] == "1" + users = User.code_of_conduct_pending + users = users.where(slack_coc_requested_at: nil) unless reminder + users = users.select(&:can_authenticate?) + + sample = users.first || User.new(first_name: "Ada", last_name: "Lovelace") + puts "Message for #{sample.email.presence || 'a sample member'}:" + puts "-" * 50 + puts CodeOfConductRequestJob.paragraphs(sample, deadline:).join("\n\n") + puts "-" * 50 + puts "#{users.size} members to ask#{' (reminder)' if reminder}." + + unless ENV["SEND"] == "1" + puts "Preview only. Run again with SEND=1 to send." + next + end + + # Space the jobs out to stay under Slack's chat.postMessage rate limit. + users.each_with_index do |user, index| + CodeOfConductRequestJob.set(wait: (index * 2).seconds).perform_later(user.id, deadline: deadline, reminder: reminder) + end + puts "Enqueued #{users.size} requests." + end + end end diff --git a/spec/factories/users.rb b/spec/factories/users.rb index a6b188a..3a2cdd9 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -28,6 +28,7 @@ # slack_birthday :date # slack_city :string # slack_coc_accepted_at :datetime +# slack_coc_requested_at :datetime # slack_cost_center :string # slack_country :string # slack_department :string diff --git a/spec/jobs/code_of_conduct_request_job_spec.rb b/spec/jobs/code_of_conduct_request_job_spec.rb new file mode 100644 index 0000000..333ad23 --- /dev/null +++ b/spec/jobs/code_of_conduct_request_job_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe CodeOfConductRequestJob do + let(:slack) { instance_double(SlackService, configured?: true, post_code_of_conduct: true) } + let!(:user) { create(:user, :verified, first_name: "ada", slack_id: "U1", slack_membership: "member") } + + before { allow(SlackService).to receive(:new).and_return(slack) } + + it "sends a Slack DM and an email, and records the request" do + expect(slack).to receive(:post_code_of_conduct) + .with("U1", intro: a_string_including("Hi Ada,", "Choose the button below"), collect_name: false) + + expect { described_class.perform_now(user.id) } + .to have_enqueued_mail(UserMailer, :code_of_conduct_request) + + expect(user.reload.slack_coc_requested_at).to be_present + end + + it "uses the form button when the name is missing" do + user.update!(first_name: "NOTSET", last_name: "NOTSET") + expect(slack).to receive(:post_code_of_conduct) + .with("U1", intro: a_string_including("Hi there,", "don't have your name"), collect_name: true) + + described_class.perform_now(user.id) + end + + it "includes the deadline when one is given" do + expect(slack).to receive(:post_code_of_conduct) + .with("U1", intro: a_string_including("Please accept by November 1, 2026."), collect_name: false) + + described_class.perform_now(user.id, deadline: Date.new(2026, 11, 1)) + end + + it "still sends the email when Slack fails" do + allow(slack).to receive(:post_code_of_conduct).and_raise(Slack::Web::Api::Errors::SlackError, "channel_not_found") + + expect { described_class.perform_now(user.id) }.to have_enqueued_mail(UserMailer, :code_of_conduct_request) + end + + it "skips someone who already accepted" do + user.update!(slack_coc_accepted_at: 1.day.ago) + + expect { described_class.perform_now(user.id) }.not_to have_enqueued_mail(UserMailer, :code_of_conduct_request) + end + + it "skips someone already asked, unless it is a reminder" do + user.update!(slack_coc_requested_at: 1.week.ago) + + expect { described_class.perform_now(user.id) }.not_to have_enqueued_mail(UserMailer, :code_of_conduct_request) + expect { described_class.perform_now(user.id, reminder: true) }.to have_enqueued_mail(UserMailer, :code_of_conduct_request) + end + + it "skips an account that can't sign in" do + user.update!(locked_at: Time.current) + + expect { described_class.perform_now(user.id) }.not_to have_enqueued_mail(UserMailer, :code_of_conduct_request) + end +end diff --git a/spec/jobs/slack_code_of_conduct_accepted_job_spec.rb b/spec/jobs/slack_code_of_conduct_accepted_job_spec.rb index 887d53e..8822896 100644 --- a/spec/jobs/slack_code_of_conduct_accepted_job_spec.rb +++ b/spec/jobs/slack_code_of_conduct_accepted_job_spec.rb @@ -33,4 +33,20 @@ expect(user.reload.slack_coc_accepted_at).to be_present expect(user).to be_slack_pending end + + it "only records acceptance for someone who is already a full member" do + user.update!(slack_membership: "member") + expect(service).not_to receive(:promote_to_member) + + described_class.perform_now("U123") + + expect(user.reload.slack_coc_accepted_at).to be_present + end + + it "replaces the DM when one is given" do + allow(service).to receive(:promote_to_member).and_return({ ok: true }) + expect(service).to receive(:mark_code_of_conduct_accepted).with(channel: "D1", ts: "1.2", already_member: false) + + described_class.perform_now("U123", message: { channel: "D1", ts: "1.2" }) + end end diff --git a/spec/mailers/previews/user_mailer_preview.rb b/spec/mailers/previews/user_mailer_preview.rb index 5bdccf9..455288e 100644 --- a/spec/mailers/previews/user_mailer_preview.rb +++ b/spec/mailers/previews/user_mailer_preview.rb @@ -10,4 +10,10 @@ def slack_email_address_added UserMailer.slack_email_address_added(EmailAddress.where(is_primary: false).first || EmailAddress.first) end + # Preview this email at http://localhost:3000/rails/mailers/user_mailer/code_of_conduct_request + def code_of_conduct_request + user = User.code_of_conduct_pending.first || User.first + UserMailer.code_of_conduct_request(user, deadline: 3.weeks.from_now.to_date) + end + end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index cc20843..a02fa21 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -39,4 +39,15 @@ expect(body).to match(/didn't change/i) end end + + describe "#code_of_conduct_request" do + subject(:mail) { described_class.code_of_conduct_request(user, deadline: Date.new(2026, 11, 1)) } + + it "asks the member to sign in and accept" do + expect(mail.to).to eq(["ada@example.com"]) + expect(mail.subject).to include("Code of Conduct") + body = mail.body.encoded + expect(body).to include("Hi Ada,", "Sign in to Weave with ada@example.com", "/login", "November 1, 2026") + end + end end diff --git a/spec/models/user_slack_membership_spec.rb b/spec/models/user_slack_membership_spec.rb index 0f660ce..5a73454 100644 --- a/spec/models/user_slack_membership_spec.rb +++ b/spec/models/user_slack_membership_spec.rb @@ -70,8 +70,48 @@ expect(user.slack_onboarding_step).to eq(:awaiting_promotion) end - it "is done for a full member" do - expect(build(:user, slack_id: "U1", slack_membership: "member").slack_onboarding_step).to eq(:member) + it "is done for a full member who accepted the code of conduct" do + user = build(:user, slack_id: "U1", slack_membership: "member", slack_coc_accepted_at: 1.day.ago) + + expect(user.slack_onboarding_step).to eq(:member) + expect(user).to be_slack_onboarding_complete + end + + it "asks a full member who joined before the code-of-conduct flow to accept it" do + user = build(:user, slack_id: "U1", slack_membership: "member") + + expect(user.slack_onboarding_step).to eq(:accept_code_of_conduct) + expect(user).not_to be_slack_onboarding_complete + end + end + + describe ".code_of_conduct_pending" do + it "finds full members without an acceptance" do + legacy = create(:user, slack_id: "U1", slack_membership: "member") + create(:user, slack_id: "U2", slack_membership: "member", slack_coc_accepted_at: 1.day.ago) + create(:user, slack_id: "U3") + create(:user) + + expect(described_class.code_of_conduct_pending).to contain_exactly(legacy) + end + end + + describe "#name_missing?" do + it "is true for the names the Slack import makes up" do + expect(build(:user, first_name: "NOTSET", last_name: "NOTSET")).to be_name_missing + expect(build(:user, first_name: "Ada", last_name: "NOTSET")).to be_name_missing + expect(build(:user, first_name: "Unknown", last_name: "User")).to be_name_missing + end + + it "is false for a real name" do + expect(build(:user, first_name: "Ada", last_name: "Lovelace")).not_to be_name_missing + end + + it "matches the name_missing scope" do + missing = create(:user, first_name: "NOTSET", last_name: "NOTSET") + create(:user, first_name: "Ada", last_name: "Lovelace") + + expect(described_class.name_missing).to contain_exactly(missing) end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e8370fa..4207f45 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -28,6 +28,7 @@ # slack_birthday :date # slack_city :string # slack_coc_accepted_at :datetime +# slack_coc_requested_at :datetime # slack_cost_center :string # slack_country :string # slack_department :string diff --git a/spec/requests/slack_interactions_spec.rb b/spec/requests/slack_interactions_spec.rb index d954350..c10016b 100644 --- a/spec/requests/slack_interactions_spec.rb +++ b/spec/requests/slack_interactions_spec.rb @@ -45,6 +45,74 @@ def signed_post(body) expect(response).to have_http_status(:ok) end + it "thanks a full member without promising new access" do + create(:user, slack_id: "U123", slack_membership: "member") + payload = { type: "block_actions", user: { id: "U123" }, actions: [{ action_id: "accept_coc", value: "U123" }] }.to_json + + signed_post("payload=#{CGI.escape(payload)}") + + expect(response.parsed_body["text"]).not_to include("full access") + end + + describe "code-of-conduct form" do + let(:slack) { instance_double(SlackService) } + let!(:user) { create(:user, slack_id: "U123", slack_membership: "member", first_name: "NOTSET", last_name: "NOTSET") } + + before { allow(SlackService).to receive(:new).and_return(slack) } + + def submission(first_name:, last_name:, user_id: "U123") + { + type: "view_submission", + user: { id: user_id }, + view: { + callback_id: "coc_form", + private_metadata: { channel: "D1", ts: "1.2" }.to_json, + state: { values: { first_name: { value: { value: first_name } }, last_name: { value: { value: last_name } } } } + } + }.to_json + end + + it "opens the form from the DM button" do + expect(slack).to receive(:open_code_of_conduct_form) + .with(trigger_id: "T1", user: user, message: { channel: "D1", ts: "1.2" }) + payload = { + type: "block_actions", trigger_id: "T1", user: { id: "U123" }, + container: { channel_id: "D1", message_ts: "1.2" }, + actions: [{ action_id: "open_coc_form", value: "U123" }] + }.to_json + + signed_post("payload=#{CGI.escape(payload)}") + + expect(response).to have_http_status(:ok) + end + + it "saves the name and accepts on submission" do + expect { signed_post("payload=#{CGI.escape(submission(first_name: "Ada", last_name: "Lovelace"))}") } + .to have_enqueued_job(SlackCodeOfConductAcceptedJob).with("U123", message: { channel: "D1", ts: "1.2" }) + + expect(response.parsed_body["response_action"]).to eq("clear") + user.reload + expect(user.full_name).to eq("Ada Lovelace") + expect(user.slack_coc_accepted_at).to be_present + end + + it "shows an error next to a missing name" do + signed_post("payload=#{CGI.escape(submission(first_name: "Ada", last_name: ""))}") + + expect(response.parsed_body).to eq("response_action" => "errors", "errors" => { "last_name" => "Enter your last name." }) + expect(user.reload.slack_coc_accepted_at).to be_nil + end + + it "uses the submitting user, so nobody can accept for someone else" do + other = create(:user, slack_id: "U999", slack_membership: "member", first_name: "Bo", last_name: "Bell") + + signed_post("payload=#{CGI.escape(submission(first_name: "Ada", last_name: "Lovelace", user_id: "U999"))}") + + expect(other.reload.slack_coc_accepted_at).to be_present + expect(user.reload.slack_coc_accepted_at).to be_nil + end + end + it "rejects a request with an invalid signature" do post "/webhooks/slack/interactions", params: "payload=%7B%7D", diff --git a/spec/requests/slack_onboarding_spec.rb b/spec/requests/slack_onboarding_spec.rb index 83c9f34..e70ca6e 100644 --- a/spec/requests/slack_onboarding_spec.rb +++ b/spec/requests/slack_onboarding_spec.rb @@ -61,13 +61,32 @@ end it "welcomes a full member" do - user.update!(slack_id: "U1", slack_membership: "member") + user.update!(slack_id: "U1", slack_membership: "member", slack_coc_accepted_at: 1.day.ago) sign_in_via_magic_link(user) get slack_onboarding_path expect(response.body).to include("You're a full member") end + + it "asks a full member who never accepted the code of conduct to accept it" do + user.update!(slack_id: "U1", slack_membership: "member") + sign_in_via_magic_link(user) + + get slack_onboarding_path + + expect(response.body).to include("now accepts our Code of Conduct", "I accept the Code of Conduct") + expect(response.body).not_to include("First name") + end + + it "asks for a name the Slack import could not find" do + user.update!(slack_id: "U1", slack_membership: "member", first_name: "NOTSET", last_name: "NOTSET") + sign_in_via_magic_link(user) + + get slack_onboarding_path + + expect(response.body).to include("First name", "Last name") + end end describe "POST /slack" do @@ -127,11 +146,42 @@ expect { post accept_code_of_conduct_slack_onboarding_path }.not_to have_enqueued_job(SlackCodeOfConductAcceptedJob) end - it "does nothing for a full member" do - user.update!(slack_id: "U1", slack_membership: "member") + it "does nothing for a full member who already accepted" do + user.update!(slack_id: "U1", slack_membership: "member", slack_coc_accepted_at: 1.day.ago) expect { post accept_code_of_conduct_slack_onboarding_path }.not_to have_enqueued_job(SlackCodeOfConductAcceptedJob) end + + it "records the acceptance for a full member who joined before the code-of-conduct flow" do + user.update!(slack_id: "U1", slack_membership: "member") + + post accept_code_of_conduct_slack_onboarding_path + + expect(user.reload.slack_coc_accepted_at).to be_present + expect(user).to be_slack_onboarding_complete + expect(response).to redirect_to(slack_onboarding_path) + end + + context "when the name is missing" do + before { user.update!(slack_id: "U1", slack_membership: "member", first_name: "NOTSET", last_name: "NOTSET") } + + it "saves the name with the acceptance" do + post accept_code_of_conduct_slack_onboarding_path, params: { first_name: "Ada", last_name: "Lovelace" } + + user.reload + expect(user.full_name).to eq("Ada Lovelace") + expect(user.slack_coc_accepted_at).to be_present + end + + it "does not accept without a name" do + post accept_code_of_conduct_slack_onboarding_path, params: { first_name: "Ada", last_name: "" } + + expect(response).to have_http_status(:unprocessable_content) + expect(response.body).to include("Enter your last name.") + expect(user.reload.slack_coc_accepted_at).to be_nil + expect(user.first_name).to eq("NOTSET") + end + end end describe "GET /" do @@ -144,12 +194,21 @@ end it "sends a full member to their account" do - user.update!(slack_id: "U1", slack_membership: "member") + user.update!(slack_id: "U1", slack_membership: "member", slack_coc_accepted_at: 1.day.ago) sign_in_via_magic_link(user) get root_path expect(response).to redirect_to(profile_path) end + + it "sends a full member who never accepted the code of conduct to onboarding" do + user.update!(slack_id: "U1", slack_membership: "member") + sign_in_via_magic_link(user) + + get root_path + + expect(response).to redirect_to(slack_onboarding_path) + end end end diff --git a/spec/serializers/user_serializer_spec.rb b/spec/serializers/user_serializer_spec.rb index 74ae70e..3fd6b56 100644 --- a/spec/serializers/user_serializer_spec.rb +++ b/spec/serializers/user_serializer_spec.rb @@ -28,6 +28,7 @@ # slack_birthday :date # slack_city :string # slack_coc_accepted_at :datetime +# slack_coc_requested_at :datetime # slack_cost_center :string # slack_country :string # slack_department :string diff --git a/spec/services/slack_webhook_service_spec.rb b/spec/services/slack_webhook_service_spec.rb index aefe8f6..b5505d4 100644 --- a/spec/services/slack_webhook_service_spec.rb +++ b/spec/services/slack_webhook_service_spec.rb @@ -207,7 +207,7 @@ it "looks the email up, links the invited guest, and sends the code of conduct" do user = create(:user, :verified, email: "guest@example.com", slack_invited_at: 1.hour.ago) allow(slack).to receive(:find_email).with("U605").and_return("guest@example.com") - expect(slack).to receive(:post_code_of_conduct).with("U605") + expect(slack).to receive(:post_code_of_conduct).with("U605", collect_name: false) described_class.process_team_join( "id" => "U605", "is_restricted" => true, "is_ultra_restricted" => true, diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index 5f82704..4f65253 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -32,6 +32,7 @@ # slack_birthday :date # slack_city :string # slack_coc_accepted_at :datetime +# slack_coc_requested_at :datetime # slack_cost_center :string # slack_country :string # slack_department :string diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 55893b9..e06733a 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -28,6 +28,7 @@ # slack_birthday :date # slack_city :string # slack_coc_accepted_at :datetime +# slack_coc_requested_at :datetime # slack_cost_center :string # slack_country :string # slack_department :string From bc79242fb91beafdb624581425b71edb9ca0b1da Mon Sep 17 00:00:00 2001 From: Jasper Mayone Date: Fri, 9 Oct 2026 13:16:28 -0400 Subject: [PATCH 2/3] feat: build code of conduct dm and form with slocks and explain data use --- Gemfile | 1 + Gemfile.lock | 4 + .../slack_onboarding_controller.rb | 19 ++- app/controllers/webhooks/slack_controller.rb | 10 +- app/jobs/code_of_conduct_request_job.rb | 8 +- app/services/slack_service.rb | 114 ++++++++---------- app/services/slack_webhook_service.rb | 2 +- .../code_of_conduct/form.slack_modal.slocks | 30 +++++ .../request.slack_message.slocks | 26 ++++ app/views/slack_onboarding/show.html.erb | 20 ++- config/locales/code_of_conduct_form.en.yml | 13 ++ config/locales/code_of_conduct_request.en.yml | 2 +- spec/jobs/code_of_conduct_request_job_spec.rb | 6 +- spec/requests/slack_interactions_spec.rb | 17 ++- spec/requests/slack_onboarding_spec.rb | 20 ++- .../slack_service_code_of_conduct_spec.rb | 70 +++++++++++ spec/services/slack_webhook_service_spec.rb | 2 +- 17 files changed, 276 insertions(+), 88 deletions(-) create mode 100644 app/views/slack/code_of_conduct/form.slack_modal.slocks create mode 100644 app/views/slack/code_of_conduct/request.slack_message.slocks create mode 100644 config/locales/code_of_conduct_form.en.yml create mode 100644 spec/services/slack_service_code_of_conduct_spec.rb diff --git a/Gemfile b/Gemfile index 8679604..f908d71 100644 --- a/Gemfile +++ b/Gemfile @@ -86,6 +86,7 @@ gem "rqrcode" # QR code generation gem "premailer-rails" # CSS to inline styles for emails gem "email_reply_parser" # Parse email replies gem "slack-ruby-client" # Slack API integration +gem "slocks", "~> 0.1.2" # Block Kit templates (.slocks views) gem "resend" # Transactional email delivery (production) ############################################################################### diff --git a/Gemfile.lock b/Gemfile.lock index efdb1c9..a7b9847 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -611,6 +611,9 @@ GEM gli hashie logger + slocks (0.1.2) + actionview (>= 6.0) + activesupport (>= 6.0) solargraph (0.57.0) backport (~> 1.2) benchmark (~> 0.4) @@ -802,6 +805,7 @@ DEPENDENCIES selenium-webdriver shoulda-matchers slack-ruby-client + slocks (~> 0.1.2) solargraph solargraph-rails (~> 1.2.4) solid_cable diff --git a/app/controllers/slack_onboarding_controller.rb b/app/controllers/slack_onboarding_controller.rb index 55e4da2..f1cc760 100644 --- a/app/controllers/slack_onboarding_controller.rb +++ b/app/controllers/slack_onboarding_controller.rb @@ -51,16 +51,19 @@ def accept_code_of_conduct return end + # "Try again" after an earlier acceptance needs no new check. + unless params[:accept] == "1" || current_user.slack_coc_accepted_at.present? + render_form_errors(accept: I18n.t("code_of_conduct_form.accept_error")) + return + end + result = CodeOfConductAcceptance.call( current_user, first_name: params[:first_name], last_name: params[:last_name] ) - unless result.success? - @step = current_user.slack_onboarding_step - @name_errors = result.errors - render :show, status: :unprocessable_content + render_form_errors(result.errors) return end @@ -72,4 +75,12 @@ def accept_code_of_conduct redirect_to slack_onboarding_path, notice: notice end + private + + def render_form_errors(errors) + @step = current_user.slack_onboarding_step + @form_errors = errors + render :show, status: :unprocessable_content + end + end diff --git a/app/controllers/webhooks/slack_controller.rb b/app/controllers/webhooks/slack_controller.rb index 4d4adf5..1501db8 100644 --- a/app/controllers/webhooks/slack_controller.rb +++ b/app/controllers/webhooks/slack_controller.rb @@ -62,7 +62,8 @@ def events # # - "accept_coc" button: accepts at once, and promotes a single-channel # guest to a full member. - # - "open_coc_form" button: opens the form that asks for a missing name. + # - "open_coc_form" button: opens the form with the box to check, which + # also asks for a missing name. # - Submission of that form: saves the name and accepts. # # Slack posts the interaction as a form-encoded `payload` field. @@ -119,6 +120,13 @@ def submit_code_of_conduct_form(payload) message = JSON.parse(payload.dig("view", "private_metadata").presence || "{}").symbolize_keys.slice(:channel, :ts).presence user = User.find_by(slack_id: slack_user_id) + # Slack makes the box required, but check it here too. + accepted = Array(values.dig("accept", "value", "selected_options")).any? { |option| option["value"] == "accept" } + unless accepted + render json: { response_action: "errors", errors: { "accept" => I18n.t("code_of_conduct_form.accept_error") } } + return + end + if user result = CodeOfConductAcceptance.call( user, diff --git a/app/jobs/code_of_conduct_request_job.rb b/app/jobs/code_of_conduct_request_job.rb index 8df1751..d824652 100644 --- a/app/jobs/code_of_conduct_request_job.rb +++ b/app/jobs/code_of_conduct_request_job.rb @@ -47,8 +47,12 @@ def send_slack_dm(user, deadline) service = SlackService.new return unless service.configured? - intro = [*self.class.paragraphs(user, deadline:), I18n.t("code_of_conduct_request.slack_action")].join("\n\n") - service.post_code_of_conduct(user.slack_id, intro: intro, collect_name: user.name_missing?) + service.post_code_of_conduct( + user.slack_id, + title: I18n.t("code_of_conduct_request.slack_title"), + paragraphs: self.class.paragraphs(user, deadline:), + form: user.name_missing? + ) rescue => e Rails.logger.error "Failed to DM the CoC request to #{user.slack_id}: #{e.message}" end diff --git a/app/services/slack_service.rb b/app/services/slack_service.rb index 4b4d042..82788e9 100644 --- a/app/services/slack_service.rb +++ b/app/services/slack_service.rb @@ -155,83 +155,76 @@ def promote_to_member(slack_user_id) end # DM someone the code of conduct with a button to accept it. Uses the bot - # token (needs chat:write + im:write). Returns the Slack API response. + # token (needs chat:write + im:write). Returns the Slack API response. The + # blocks come from app/views/slack/code_of_conduct/request.slack_message.slocks. # - # intro is the opening text (mrkdwn). It defaults to the welcome for a - # newly-joined guest. CodeOfConductRequestJob passes its own. + # title and paragraphs (mrkdwn) open the message. They default to the + # welcome for a newly-joined guest. CodeOfConductRequestJob passes its own. # - # With collect_name: false, the button accepts at once: its action_id is - # "accept_coc" and its value is the Slack user id. With collect_name: true - # (for people whose name the Slack import could not find), the button - # ("open_coc_form") opens a form that asks for their name and then accepts - # (see #open_code_of_conduct_form). The interactions webhook handles both. - def post_code_of_conduct(slack_user_id, coc_url: nil, intro: nil, collect_name: false) + # With form: false, the button accepts at once: its action_id is + # "accept_coc" and its value is the Slack user id. With form: true, the + # button ("open_coc_form") opens a form with a box to check, and name fields + # when the name is missing (see #open_code_of_conduct_form). Use the form for + # people whose name the Slack import could not find. The interactions + # webhook handles both. + def post_code_of_conduct(slack_user_id, title: nil, paragraphs: nil, form: false) raise ConfigurationError, "Slack client not configured" unless @client - coc_url ||= self.class.code_of_conduct_url - intro ||= "*Welcome to Patchwork Labs! :wave: Before you get full access to the community, " \ - "please read our Code of Conduct and accept it below.*" - coc_line = coc_url.present? ? "Read it here: #{coc_url}" : "Please review our Code of Conduct." - - button = if collect_name - { text: "Review and accept", action_id: "open_coc_form" } + title ||= "Welcome to Patchwork Labs! ๐Ÿ‘‹" + paragraphs ||= ["Before you get full access to the community, please read our Code of Conduct and accept it."] + button = if form + { text: "Review and accept", action_id: "open_coc_form", value: slack_user_id } else - { text: "I accept the Code of Conduct", action_id: "accept_coc" } + { text: "I accept the Code of Conduct", action_id: "accept_coc", value: slack_user_id } end + message = self.class.render_slack_view( + "request", :slack_message, + title: title, + paragraphs: paragraphs, + action_text: I18n.t(form ? "code_of_conduct_form.slack_form_action" : "code_of_conduct_form.slack_action"), + button: button, + coc_url: self.class.code_of_conduct_url, + web_url: Rails.application.routes.url_helpers.slack_onboarding_url(**Rails.application.config.action_mailer.default_url_options) + ) + @client.chat_postMessage( channel: slack_user_id, text: "Please review and accept the Patchwork Labs Code of Conduct.", - blocks: [ - { type: "section", text: { type: "mrkdwn", text: "#{intro}\n\n#{coc_line}" } }, - { - type: "actions", - elements: [ - { - type: "button", - style: "primary", - text: { type: "plain_text", text: button[:text], emoji: true }, - action_id: button[:action_id], - value: slack_user_id - } - ] - } - ] + blocks: message[:blocks] ) end # Opens the code-of-conduct form from a button click. trigger_id comes from # the click and expires after 3 seconds, so call this while answering it. + # The view comes from app/views/slack/code_of_conduct/form.slack_modal.slocks. # - # The form asks for a first and last name when the user's name is missing - # (or when there is no linked user, since nothing is known about them). - # message is the DM the button was in, as { channel:, ts: }. It goes into - # private_metadata, so the submission can replace the DM. + # The form has a required "I have read and accept" box, and says how Weave + # uses what it collects. It also asks for a first and last name when the + # user's name is missing (or when there is no linked user, since nothing is + # known about them). message is the DM the button was in, as + # { channel:, ts: }. It goes into private_metadata, so the submission can + # replace the DM. def open_code_of_conduct_form(trigger_id:, user:, message: nil) raise ConfigurationError, "Slack client not configured" unless @client - coc_url = self.class.code_of_conduct_url - coc_text = coc_url.present? ? "<#{coc_url}|Read the Code of Conduct>" : "Please review our Code of Conduct." - blocks = [ - { type: "section", text: { type: "mrkdwn", text: "#{coc_text}, then choose *I accept* below." } } - ] - if user.nil? || user.name_missing? - blocks.unshift({ type: "section", text: { type: "mrkdwn", text: "We don't have your name yet. Please add it." } }) - blocks.insert(1, name_input("first_name", "First name"), name_input("last_name", "Last name")) - end + view = self.class.render_slack_view( + "form", :slack_modal, + ask_name: user.nil? || user.name_missing?, + coc_url: self.class.code_of_conduct_url, + private_metadata: (message || {}).to_json + ) - @client.views_open( - trigger_id: trigger_id, - view: { - type: "modal", - callback_id: CODE_OF_CONDUCT_FORM, - private_metadata: (message || {}).to_json, - title: { type: "plain_text", text: "Code of Conduct" }, - submit: { type: "plain_text", text: "I accept" }, - close: { type: "plain_text", text: "Not now" }, - blocks: blocks - } + @client.views_open(trigger_id: trigger_id, view: view) + end + + # Renders a Block Kit template from app/views/slack/code_of_conduct with + # slocks, and returns it as a Hash with symbol keys. + def self.render_slack_view(template, format, **assigns) + json = ApplicationController.render( + template: "slack/code_of_conduct/#{template}", formats: [format], layout: false, assigns: assigns ) + JSON.parse(json, symbolize_names: true) end # Replaces a code-of-conduct DM with a thank-you, so its button can't be @@ -585,15 +578,6 @@ def workspace_subdomain @workspace_subdomain ||= ENV["SLACK_WORKSPACE_SUBDOMAIN"] || Rails.application.credentials.dig(:slack, :workspace_subdomain) end - def name_input(block_id, label) - { - type: "input", - block_id: block_id, - label: { type: "plain_text", text: label }, - element: { type: "plain_text_input", action_id: "value", max_length: 100 } - } - end - # Channel single-channel guests are invited into (the code-of-conduct channel). def coc_channel @coc_channel ||= ENV["SLACK_COC_CHANNEL"] || Rails.application.credentials.dig(:slack, :coc_channel) diff --git a/app/services/slack_webhook_service.rb b/app/services/slack_webhook_service.rb index 054034f..de5c181 100644 --- a/app/services/slack_webhook_service.rb +++ b/app/services/slack_webhook_service.rb @@ -143,7 +143,7 @@ def send_code_of_conduct(user) return if user&.slack_id.blank? return if user.slack_coc_accepted_at.present? - SlackService.new.post_code_of_conduct(user.slack_id, collect_name: user.name_missing?) + SlackService.new.post_code_of_conduct(user.slack_id, form: user.name_missing?) rescue => e Rails.logger.error "[SlackWebhookService] Failed to post CoC to #{user&.slack_id}: #{e.message}" end diff --git a/app/views/slack/code_of_conduct/form.slack_modal.slocks b/app/views/slack/code_of_conduct/form.slack_modal.slocks new file mode 100644 index 0000000..38e3fca --- /dev/null +++ b/app/views/slack/code_of_conduct/form.slack_modal.slocks @@ -0,0 +1,30 @@ +# The Slack form that accepts the code of conduct +# (SlackService#open_code_of_conduct_form). +# +# @ask_name true to show the name fields +# @coc_url the Code of Conduct, or nil +# @private_metadata JSON string + +title "Code of Conduct" +submit "I accept" +close "Not now" +callback SlackService::CODE_OF_CONDUCT_FORM +private_metadata @private_metadata + +if @coc_url.present? + section t("code_of_conduct_form.intro"), markdown: true, accessory: button("Read it", "read_coc", url: @coc_url) +else + section t("code_of_conduct_form.intro"), markdown: true +end + +if @ask_name + divider + section t("code_of_conduct_form.name_why"), markdown: true + input "First name", plain_text_input("value", max_length: 100), block_id: "first_name" + input "Last name", plain_text_input("value", max_length: 100), block_id: "last_name" +end + +divider +input "Code of Conduct", checkboxes("value", [option(t("code_of_conduct_form.accept_label"), "accept")]), block_id: "accept" + +context [mrkdwn_text(":lock: *#{t('code_of_conduct_form.data_use_title')}*\n#{t('code_of_conduct_form.data_use')}")] diff --git a/app/views/slack/code_of_conduct/request.slack_message.slocks b/app/views/slack/code_of_conduct/request.slack_message.slocks new file mode 100644 index 0000000..e522bbe --- /dev/null +++ b/app/views/slack/code_of_conduct/request.slack_message.slocks @@ -0,0 +1,26 @@ +# The Slack DM that asks someone to accept the code of conduct +# (SlackService#post_code_of_conduct). +# +# @title header text +# @paragraphs plain-text paragraphs (mrkdwn is allowed) +# @action_text what to do with the button +# @button { text:, action_id:, value: } +# @coc_url the Code of Conduct, or nil +# @web_url the /slack page, where people can also accept + +header @title + +@paragraphs.each { |paragraph| section paragraph, markdown: true } + +if @coc_url.present? + section ":scroll: *Code of Conduct*\nIt explains how we treat each other in Patchwork Labs.", + markdown: true, + accessory: button("Read it", "read_coc", url: @coc_url) +end + +divider + +section @action_text, markdown: true +actions [button(@button[:text], @button[:action_id], value: @button[:value], style: "primary")] + +context [mrkdwn_text("You can also accept on Weave: <#{@web_url}|#{@web_url.delete_prefix('https://')}>")] diff --git a/app/views/slack_onboarding/show.html.erb b/app/views/slack_onboarding/show.html.erb index a0305f1..c37cd96 100644 --- a/app/views/slack_onboarding/show.html.erb +++ b/app/views/slack_onboarding/show.html.erb @@ -85,20 +85,34 @@ <% end %> <%= form_with url: accept_code_of_conduct_slack_onboarding_path, method: :post, class: "space-y-4 text-left" do |form| %> <% if current_user.name_missing? %> -

We don't have your name yet. Please add it.

+

<%= t("code_of_conduct_form.name_why") %>

<% [[:first_name, "First name"], [:last_name, "Last name"]].each do |field, label| %>
<%= form.label field, label, class: "block text-sm font-medium text-ink" %> <%= form.text_field field, value: params[field], required: true, maxlength: 100, autocomplete: field == :first_name ? "given-name" : "family-name", class: "mt-1 w-full px-4 py-3 rounded-lg border-2 border-neutral-300 bg-paper text-ink focus:ring-2 focus:ring-grape focus:border-grape transition-colors" %> - <% if @name_errors&.key?(field) %> -

<%= @name_errors[field] %>

+ <% if @form_errors&.key?(field) %> +

<%= @form_errors[field] %>

<% end %>
<% end %> <% end %> +
+ + <% if @form_errors&.key?(:accept) %> +

<%= @form_errors[:accept] %>

+ <% end %> +
<%= form.submit "I accept the Code of Conduct", class: "btn-stitch w-full justify-center" %> +
+

<%= t("code_of_conduct_form.data_use_title") %>

+

<%= t("code_of_conduct_form.data_use") %>

+
<% end %> <% when :awaiting_promotion %> diff --git a/config/locales/code_of_conduct_form.en.yml b/config/locales/code_of_conduct_form.en.yml new file mode 100644 index 0000000..d40b6c9 --- /dev/null +++ b/config/locales/code_of_conduct_form.en.yml @@ -0,0 +1,13 @@ +# The form people use to accept the code of conduct: the Slack form (see +# app/views/slack/code_of_conduct/form.slack_modal.slocks) and the /slack page. +# Both tell people why we ask for their data and how we use it. +en: + code_of_conduct_form: + slack_action: "Choose the button below to accept." + slack_form_action: "Choose *Review and accept*, then check the box." + intro: "Read the Code of Conduct, then check the box below to accept it." + accept_label: "I have read and accept the Code of Conduct" + accept_error: "Check the box to accept the Code of Conduct." + name_why: "Your Slack profile has no name, so we don't know what to call you. Please add your first and last name." + data_use_title: "How we use this information" + data_use: "We save the date and time you accept in your Weave account, so we have a record that you accepted. If you add your name, we save it in your Weave account too. Weave admins can see this information. Patchwork Labs apps that you can use with Weave can see your name." diff --git a/config/locales/code_of_conduct_request.en.yml b/config/locales/code_of_conduct_request.en.yml index 1267a2a..3a23c26 100644 --- a/config/locales/code_of_conduct_request.en.yml +++ b/config/locales/code_of_conduct_request.en.yml @@ -14,6 +14,6 @@ en: New members now join the Slack through Weave, and they accept our Code of Conduct when they join. You joined before we added that step, so we ask you to accept it now. It takes a few seconds. name_missing: "We also don't have your name yet. Please add it when you accept." deadline: "Please accept by %{deadline}. After that date, members who have not accepted move to guest access in the Slack until they accept." - slack_action: "Choose the button below to accept." + slack_title: "๐Ÿ“œ Please accept our Code of Conduct" email_action: "Sign in to Weave with %{email} to accept." sign_off: "Thanks for being part of Patchwork Labs!" diff --git a/spec/jobs/code_of_conduct_request_job_spec.rb b/spec/jobs/code_of_conduct_request_job_spec.rb index 333ad23..0eed6a8 100644 --- a/spec/jobs/code_of_conduct_request_job_spec.rb +++ b/spec/jobs/code_of_conduct_request_job_spec.rb @@ -10,7 +10,7 @@ it "sends a Slack DM and an email, and records the request" do expect(slack).to receive(:post_code_of_conduct) - .with("U1", intro: a_string_including("Hi Ada,", "Choose the button below"), collect_name: false) + .with("U1", title: a_string_including("Code of Conduct"), paragraphs: include("Hi Ada,"), form: false) expect { described_class.perform_now(user.id) } .to have_enqueued_mail(UserMailer, :code_of_conduct_request) @@ -21,14 +21,14 @@ it "uses the form button when the name is missing" do user.update!(first_name: "NOTSET", last_name: "NOTSET") expect(slack).to receive(:post_code_of_conduct) - .with("U1", intro: a_string_including("Hi there,", "don't have your name"), collect_name: true) + .with("U1", title: anything, paragraphs: include("Hi there,", a_string_including("don't have your name")), form: true) described_class.perform_now(user.id) end it "includes the deadline when one is given" do expect(slack).to receive(:post_code_of_conduct) - .with("U1", intro: a_string_including("Please accept by November 1, 2026."), collect_name: false) + .with("U1", title: anything, paragraphs: include(a_string_including("Please accept by November 1, 2026.")), form: false) described_class.perform_now(user.id, deadline: Date.new(2026, 11, 1)) end diff --git a/spec/requests/slack_interactions_spec.rb b/spec/requests/slack_interactions_spec.rb index c10016b..57733a7 100644 --- a/spec/requests/slack_interactions_spec.rb +++ b/spec/requests/slack_interactions_spec.rb @@ -60,14 +60,20 @@ def signed_post(body) before { allow(SlackService).to receive(:new).and_return(slack) } - def submission(first_name:, last_name:, user_id: "U123") + def submission(first_name:, last_name:, user_id: "U123", accept: true) { type: "view_submission", user: { id: user_id }, view: { callback_id: "coc_form", private_metadata: { channel: "D1", ts: "1.2" }.to_json, - state: { values: { first_name: { value: { value: first_name } }, last_name: { value: { value: last_name } } } } + state: { + values: { + accept: { value: { selected_options: accept ? [{ value: "accept" }] : [] } }, + first_name: { value: { value: first_name } }, + last_name: { value: { value: last_name } } + } + } } }.to_json end @@ -103,6 +109,13 @@ def submission(first_name:, last_name:, user_id: "U123") expect(user.reload.slack_coc_accepted_at).to be_nil end + it "does not accept without the box checked" do + signed_post("payload=#{CGI.escape(submission(first_name: "Ada", last_name: "Lovelace", accept: false))}") + + expect(response.parsed_body["errors"]).to have_key("accept") + expect(user.reload.slack_coc_accepted_at).to be_nil + end + it "uses the submitting user, so nobody can accept for someone else" do other = create(:user, slack_id: "U999", slack_membership: "member", first_name: "Bo", last_name: "Bell") diff --git a/spec/requests/slack_onboarding_spec.rb b/spec/requests/slack_onboarding_spec.rb index e70ca6e..9b7223f 100644 --- a/spec/requests/slack_onboarding_spec.rb +++ b/spec/requests/slack_onboarding_spec.rb @@ -75,7 +75,7 @@ get slack_onboarding_path - expect(response.body).to include("now accepts our Code of Conduct", "I accept the Code of Conduct") + expect(response.body).to include("now accepts our Code of Conduct", "I have read and accept", "How we use this information") expect(response.body).not_to include("First name") end @@ -131,7 +131,7 @@ it "accepts the code of conduct for a guest" do user.update!(slack_id: "U1") - expect { post accept_code_of_conduct_slack_onboarding_path } + expect { post accept_code_of_conduct_slack_onboarding_path, params: { accept: "1" } } .to have_enqueued_job(SlackCodeOfConductAcceptedJob).with("U1") end @@ -142,6 +142,16 @@ .to have_enqueued_job(SlackCodeOfConductAcceptedJob).with("U1") end + it "does not accept without the box checked" do + user.update!(slack_id: "U1", slack_membership: "member") + + post accept_code_of_conduct_slack_onboarding_path + + expect(response).to have_http_status(:unprocessable_content) + expect(response.body).to include("Check the box") + expect(user.reload.slack_coc_accepted_at).to be_nil + end + it "does nothing for someone who isn't in the Slack yet" do expect { post accept_code_of_conduct_slack_onboarding_path }.not_to have_enqueued_job(SlackCodeOfConductAcceptedJob) end @@ -155,7 +165,7 @@ it "records the acceptance for a full member who joined before the code-of-conduct flow" do user.update!(slack_id: "U1", slack_membership: "member") - post accept_code_of_conduct_slack_onboarding_path + post accept_code_of_conduct_slack_onboarding_path, params: { accept: "1" } expect(user.reload.slack_coc_accepted_at).to be_present expect(user).to be_slack_onboarding_complete @@ -166,7 +176,7 @@ before { user.update!(slack_id: "U1", slack_membership: "member", first_name: "NOTSET", last_name: "NOTSET") } it "saves the name with the acceptance" do - post accept_code_of_conduct_slack_onboarding_path, params: { first_name: "Ada", last_name: "Lovelace" } + post accept_code_of_conduct_slack_onboarding_path, params: { accept: "1", first_name: "Ada", last_name: "Lovelace" } user.reload expect(user.full_name).to eq("Ada Lovelace") @@ -174,7 +184,7 @@ end it "does not accept without a name" do - post accept_code_of_conduct_slack_onboarding_path, params: { first_name: "Ada", last_name: "" } + post accept_code_of_conduct_slack_onboarding_path, params: { accept: "1", first_name: "Ada", last_name: "" } expect(response).to have_http_status(:unprocessable_content) expect(response.body).to include("Enter your last name.") diff --git a/spec/services/slack_service_code_of_conduct_spec.rb b/spec/services/slack_service_code_of_conduct_spec.rb new file mode 100644 index 0000000..fa4e72e --- /dev/null +++ b/spec/services/slack_service_code_of_conduct_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require "rails_helper" + +# The code-of-conduct DM and form are Block Kit views rendered with slocks +# (app/views/slack/code_of_conduct). +RSpec.describe SlackService do + subject(:service) { described_class.new } + + let(:client) { instance_double(Slack::Web::Client) } + + before do + service.instance_variable_set(:@client, client) + allow(described_class).to receive(:code_of_conduct_url).and_return("https://patchworklabs.org/coc") + end + + def block_types(blocks) = blocks.map { |block| block[:type] } + + describe "#post_code_of_conduct" do + it "sends a rich DM with the paragraphs, a link to the Code of Conduct, and an accept button" do + expect(client).to receive(:chat_postMessage) do |args| + blocks = args[:blocks] + expect(block_types(blocks)).to eq(%w[header section section divider section actions context]) + expect(blocks.first(2).map { |block| block[:text][:text] }).to eq(["Hello", "First paragraph"]) + expect(blocks[2][:accessory][:url]).to eq("https://patchworklabs.org/coc") + expect(blocks[5][:elements].first).to include(action_id: "accept_coc", value: "U1", style: "primary") + expect(blocks[6][:elements].first[:text]).to include("/slack") + end + + service.post_code_of_conduct("U1", title: "Hello", paragraphs: ["First paragraph"]) + end + + it "uses the form button when asked" do + expect(client).to receive(:chat_postMessage) do |args| + button = args[:blocks].find { |block| block[:type] == "actions" }[:elements].first + expect(button[:action_id]).to eq("open_coc_form") + end + + service.post_code_of_conduct("U1", form: true) + end + end + + describe "#open_code_of_conduct_form" do + let(:user) { build(:user, first_name: "NOTSET", last_name: "NOTSET") } + + it "opens a form with a required box, name fields for a missing name, and how the data is used" do + expect(client).to receive(:views_open) do |args| + view = args[:view] + expect(args[:trigger_id]).to eq("T1") + expect(view).to include(type: "modal", callback_id: "coc_form", private_metadata: { channel: "D1", ts: "1.2" }.to_json) + inputs = view[:blocks].select { |block| block[:type] == "input" } + expect(inputs.map { |block| block[:block_id] }).to eq(%w[first_name last_name accept]) + expect(inputs.last[:element][:type]).to eq("checkboxes") + expect(view[:blocks].last[:elements].first[:text]).to include("How we use this information") + end + + service.open_code_of_conduct_form(trigger_id: "T1", user: user, message: { channel: "D1", ts: "1.2" }) + end + + it "leaves the name fields out when the name is known" do + user.assign_attributes(first_name: "Ada", last_name: "Lovelace") + expect(client).to receive(:views_open) do |args| + block_ids = args[:view][:blocks].filter_map { |block| block[:block_id] } + expect(block_ids).to eq(%w[accept]) + end + + service.open_code_of_conduct_form(trigger_id: "T1", user: user) + end + end +end diff --git a/spec/services/slack_webhook_service_spec.rb b/spec/services/slack_webhook_service_spec.rb index b5505d4..cb12774 100644 --- a/spec/services/slack_webhook_service_spec.rb +++ b/spec/services/slack_webhook_service_spec.rb @@ -207,7 +207,7 @@ it "looks the email up, links the invited guest, and sends the code of conduct" do user = create(:user, :verified, email: "guest@example.com", slack_invited_at: 1.hour.ago) allow(slack).to receive(:find_email).with("U605").and_return("guest@example.com") - expect(slack).to receive(:post_code_of_conduct).with("U605", collect_name: false) + expect(slack).to receive(:post_code_of_conduct).with("U605", form: false) described_class.process_team_join( "id" => "U605", "is_restricted" => true, "is_ultra_restricted" => true, From 6ad08ff8ae749a0f9d2d92924b8a5f30ad9a3781 Mon Sep 17 00:00:00 2001 From: Jasper Mayone Date: Fri, 9 Oct 2026 13:30:58 -0400 Subject: [PATCH 3/3] feat: ask for preferred, legal and slack names, and use plain block kit --- Gemfile | 1 - Gemfile.lock | 4 - .../slack_onboarding_controller.rb | 3 +- app/controllers/webhooks/slack_controller.rb | 3 +- app/jobs/push_slack_name_job.rb | 28 ++++++ app/services/code_of_conduct_acceptance.rb | 47 ++++++--- app/services/code_of_conduct_slack_views.rb | 99 +++++++++++++++++++ app/services/slack_service.rb | 43 ++++---- .../code_of_conduct/form.slack_modal.slocks | 30 ------ .../request.slack_message.slocks | 26 ----- app/views/slack_onboarding/show.html.erb | 12 ++- config/locales/code_of_conduct_form.en.yml | 11 ++- spec/jobs/push_slack_name_job_spec.rb | 29 ++++++ spec/requests/slack_interactions_spec.rb | 14 ++- spec/requests/slack_onboarding_spec.rb | 26 ++++- .../slack_service_code_of_conduct_spec.rb | 9 +- 16 files changed, 271 insertions(+), 114 deletions(-) create mode 100644 app/jobs/push_slack_name_job.rb create mode 100644 app/services/code_of_conduct_slack_views.rb delete mode 100644 app/views/slack/code_of_conduct/form.slack_modal.slocks delete mode 100644 app/views/slack/code_of_conduct/request.slack_message.slocks create mode 100644 spec/jobs/push_slack_name_job_spec.rb diff --git a/Gemfile b/Gemfile index f908d71..8679604 100644 --- a/Gemfile +++ b/Gemfile @@ -86,7 +86,6 @@ gem "rqrcode" # QR code generation gem "premailer-rails" # CSS to inline styles for emails gem "email_reply_parser" # Parse email replies gem "slack-ruby-client" # Slack API integration -gem "slocks", "~> 0.1.2" # Block Kit templates (.slocks views) gem "resend" # Transactional email delivery (production) ############################################################################### diff --git a/Gemfile.lock b/Gemfile.lock index a7b9847..efdb1c9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -611,9 +611,6 @@ GEM gli hashie logger - slocks (0.1.2) - actionview (>= 6.0) - activesupport (>= 6.0) solargraph (0.57.0) backport (~> 1.2) benchmark (~> 0.4) @@ -805,7 +802,6 @@ DEPENDENCIES selenium-webdriver shoulda-matchers slack-ruby-client - slocks (~> 0.1.2) solargraph solargraph-rails (~> 1.2.4) solid_cable diff --git a/app/controllers/slack_onboarding_controller.rb b/app/controllers/slack_onboarding_controller.rb index f1cc760..b05d6cf 100644 --- a/app/controllers/slack_onboarding_controller.rb +++ b/app/controllers/slack_onboarding_controller.rb @@ -59,8 +59,7 @@ def accept_code_of_conduct result = CodeOfConductAcceptance.call( current_user, - first_name: params[:first_name], - last_name: params[:last_name] + names: params.slice(*CodeOfConductAcceptance::NAME_FIELDS).permit(*CodeOfConductAcceptance::NAME_FIELDS).to_h.symbolize_keys ) unless result.success? render_form_errors(result.errors) diff --git a/app/controllers/webhooks/slack_controller.rb b/app/controllers/webhooks/slack_controller.rb index 1501db8..388f0a2 100644 --- a/app/controllers/webhooks/slack_controller.rb +++ b/app/controllers/webhooks/slack_controller.rb @@ -130,8 +130,7 @@ def submit_code_of_conduct_form(payload) if user result = CodeOfConductAcceptance.call( user, - first_name: values.dig("first_name", "value", "value"), - last_name: values.dig("last_name", "value", "value"), + names: CodeOfConductAcceptance::NAME_FIELDS.index_with { |field| values.dig(field.to_s, "value", "value") }, message: message ) unless result.success? diff --git a/app/jobs/push_slack_name_job.rb b/app/jobs/push_slack_name_job.rb new file mode 100644 index 0000000..383ba28 --- /dev/null +++ b/app/jobs/push_slack_name_job.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +# Sets the name on a user's Slack profile after they give it on the +# code-of-conduct form (see CodeOfConductAcceptance). With a nickname, only +# the Slack display name is set, so the preferred name stays out of Slack. +# Without one, the Slack first and last name are set to the preferred name. +# Slack errors and rate limits are retried. +class PushSlackNameJob < ApplicationJob + queue_as :default + + retry_on Slack::Web::Api::Errors::TooManyRequestsError, attempts: 10, + wait: ->(executions) { [executions * 30, 300].min } + retry_on Slack::Web::Api::Errors::SlackError, Slack::Web::Api::Errors::ServerError, + wait: :polynomially_longer, attempts: 5 + + def perform(user_id, nickname: nil) + user = User.find_by(id: user_id) + return if user&.slack_id.blank? + + profile = if nickname.present? + { display_name: nickname } + else + { first_name: user.first_name, last_name: user.last_name } + end + SlackService.new.update_slack_profile_name(user.slack_id, **profile) + end + +end diff --git a/app/services/code_of_conduct_acceptance.rb b/app/services/code_of_conduct_acceptance.rb index 815c859..84e204f 100644 --- a/app/services/code_of_conduct_acceptance.rb +++ b/app/services/code_of_conduct_acceptance.rb @@ -4,25 +4,30 @@ # the Slack form (see Webhooks::SlackController#interactions). # # When the Slack import gave the person a placeholder name (User#name_missing?), -# they must also give their real name. The acceptance is recorded here, at -# once, so the next page they see is up to date. SlackCodeOfConductAcceptedJob -# then does the Slack side: it promotes a guest to full member, and it updates -# the Slack DM when one is given. +# they must also give their preferred name. They can also give a legal name +# (only if it differs) and a Slack nickname (if they don't want their +# preferred name in Slack). The Slack profile then gets the nickname, or the +# preferred name (see PushSlackNameJob). +# +# The acceptance is recorded here, at once, so the next page they see is up +# to date. SlackCodeOfConductAcceptedJob then does the Slack side: it promotes +# a guest to full member, and it updates the Slack DM when one is given. class CodeOfConductAcceptance Result = Data.define(:user, :errors) do def success? = errors.empty? end - # first_name / last_name are read only when the user's name is missing. - # message is the Slack DM to update, as { channel:, ts: }. - def self.call(user, first_name: nil, last_name: nil, message: nil) - new(user, first_name:, last_name:, message:).call + NAME_FIELDS = %i[first_name last_name legal_first_name legal_last_name slack_name].freeze + + # names holds the NAME_FIELDS. They are read only when the user's name is + # missing. message is the Slack DM to update, as { channel:, ts: }. + def self.call(user, names: {}, message: nil) + new(user, names:, message:).call end - def initialize(user, first_name:, last_name:, message:) + def initialize(user, names:, message:) @user = user - @first_name = first_name.to_s.strip - @last_name = last_name.to_s.strip + @names = NAME_FIELDS.index_with { |field| names[field].to_s.strip } @message = message end @@ -31,7 +36,7 @@ def call errors = name_errors return Result.new(user: @user, errors:) if errors.any? - @user.update!(first_name: @first_name, last_name: @last_name) + save_names end @user.update_columns(slack_coc_accepted_at: Time.current) if @user.slack_coc_accepted_at.nil? # rubocop:disable Rails/SkipsModelValidations @@ -45,8 +50,12 @@ def call # Keyed by field, so the Slack form can show each error next to its input. def name_errors errors = {} - errors[:first_name] = "Enter your first name." if real_name_part(@first_name).nil? - errors[:last_name] = "Enter your last name." if real_name_part(@last_name).nil? + errors[:first_name] = "Enter your preferred first name." if real_name_part(@names[:first_name]).nil? + errors[:last_name] = "Enter your preferred last name." if real_name_part(@names[:last_name]).nil? + if @names[:legal_first_name].present? ^ @names[:legal_last_name].present? + errors[@names[:legal_first_name].present? ? :legal_last_name : :legal_first_name] = "Enter both parts of your legal name, or leave both blank." + end + errors[:slack_name] = "Use 80 characters or fewer." if @names[:slack_name].length > 80 errors end @@ -54,4 +63,14 @@ def real_name_part(value) value.presence unless User::PLACEHOLDER_NAMES.include?(value) end + def save_names + @user.update!( + first_name: @names[:first_name], + last_name: @names[:last_name], + legal_first_name: @names[:legal_first_name].presence, + legal_last_name: @names[:legal_last_name].presence + ) + PushSlackNameJob.perform_later(@user.id, nickname: @names[:slack_name].presence) if @user.slack_id.present? + end + end diff --git a/app/services/code_of_conduct_slack_views.rb b/app/services/code_of_conduct_slack_views.rb new file mode 100644 index 0000000..f7f49f7 --- /dev/null +++ b/app/services/code_of_conduct_slack_views.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +# Block Kit payloads for the code-of-conduct DM and form (see +# SlackService#post_code_of_conduct and #open_code_of_conduct_form). Plain +# hashes, as Slack's Block Kit reference documents them: +# https://docs.slack.dev/reference/block-kit/blocks +module CodeOfConductSlackViews + # callback_id of the form. The interactions webhook routes on it. + FORM_CALLBACK = "coc_form" + + # block_id of each field on the form. Each input's action_id is "value". + # A pair of fields shares one hint, under its second field. The /slack page + # uses the same list. + NAME_FIELDS = { + first_name: { label: "Preferred first name", hint_for: :last_name }, + last_name: { label: "Preferred last name", hint: :preferred_hint }, + legal_first_name: { label: "Legal first name", optional: true, hint_for: :legal_last_name }, + legal_last_name: { label: "Legal last name", hint: :legal_hint, optional: true }, + slack_name: { label: "Slack nickname", hint: :slack_name_hint, optional: true, max_length: 80 } + }.freeze + + module_function + + # The DM. button is { text:, action_id:, value: }. + def request(title:, paragraphs:, action_text:, button:, coc_url:, web_url:) + blocks = [header(title), *paragraphs.map { |paragraph| section(paragraph) }] + if coc_url.present? + blocks << section(":scroll: *Code of Conduct*\nIt explains how we treat each other in Patchwork Labs.", + accessory: link_button("Read it", coc_url)) + end + blocks << { type: "divider" } + blocks << section(action_text) + blocks << { + type: "actions", + elements: [{ type: "button", style: "primary", text: plain(button[:text]), action_id: button[:action_id], value: button[:value] }] + } + blocks << context("You can also accept on Weave: <#{web_url}|#{web_url.delete_prefix('https://')}>") + { blocks: blocks } + end + + # The form. ask_name adds the name fields. + def form(ask_name:, coc_url:, private_metadata:) + intro = t(:intro) + blocks = [coc_url.present? ? section(intro, accessory: link_button("Read it", coc_url)) : section(intro)] + + if ask_name + blocks << { type: "divider" } + blocks << section(t(:name_why)) + NAME_FIELDS.each { |block_id, field| blocks << text_input(block_id, **field) } + end + + blocks << { type: "divider" } + blocks << { + type: "input", + block_id: "accept", + label: plain("Code of Conduct"), + element: { + type: "checkboxes", + action_id: "value", + options: [{ text: plain(t(:accept_label)), value: "accept" }] + } + } + blocks << context(":lock: *#{t(:data_use_title)}*\n#{t(:data_use)}") + + { + type: "modal", + callback_id: FORM_CALLBACK, + private_metadata: private_metadata, + title: plain("Code of Conduct"), + submit: plain("I accept"), + close: plain("Not now"), + blocks: blocks + } + end + + def t(key) = I18n.t("code_of_conduct_form.#{key}") + def plain(text) = { type: "plain_text", text: text } + def header(text) = { type: "header", text: plain(text) } + def context(text) = { type: "context", elements: [{ type: "mrkdwn", text: text }] } + + def section(text, accessory: nil) + { type: "section", text: { type: "mrkdwn", text: text }, accessory: accessory }.compact + end + + # A URL button still sends a click to the interactions webhook, which + # ignores the action_id. + def link_button(text, url) = { type: "button", text: plain(text), action_id: "read_coc", url: url } + + def text_input(block_id, label:, hint: nil, optional: false, max_length: 100, hint_for: nil) + { + type: "input", + block_id: block_id.to_s, + optional: optional, + label: plain(label), + hint: (plain(t(hint)) if hint), + element: { type: "plain_text_input", action_id: "value", max_length: max_length } + }.compact + end +end diff --git a/app/services/slack_service.rb b/app/services/slack_service.rb index 82788e9..cdd7825 100644 --- a/app/services/slack_service.rb +++ b/app/services/slack_service.rb @@ -40,7 +40,7 @@ class ApiError < SlackError; end PWL_ID_FIELD = "pwl_idp_id" # callback_id of the code-of-conduct form (see #open_code_of_conduct_form). - CODE_OF_CONDUCT_FORM = "coc_form" + CODE_OF_CONDUCT_FORM = CodeOfConductSlackViews::FORM_CALLBACK # How long a workspace's label -> field ID map is cached. CUSTOM_FIELD_IDS_TTL = 1.hour @@ -156,7 +156,7 @@ def promote_to_member(slack_user_id) # DM someone the code of conduct with a button to accept it. Uses the bot # token (needs chat:write + im:write). Returns the Slack API response. The - # blocks come from app/views/slack/code_of_conduct/request.slack_message.slocks. + # blocks come from CodeOfConductSlackViews.request. # # title and paragraphs (mrkdwn) open the message. They default to the # welcome for a newly-joined guest. CodeOfConductRequestJob passes its own. @@ -178,8 +178,7 @@ def post_code_of_conduct(slack_user_id, title: nil, paragraphs: nil, form: false { text: "I accept the Code of Conduct", action_id: "accept_coc", value: slack_user_id } end - message = self.class.render_slack_view( - "request", :slack_message, + message = CodeOfConductSlackViews.request( title: title, paragraphs: paragraphs, action_text: I18n.t(form ? "code_of_conduct_form.slack_form_action" : "code_of_conduct_form.slack_action"), @@ -197,19 +196,18 @@ def post_code_of_conduct(slack_user_id, title: nil, paragraphs: nil, form: false # Opens the code-of-conduct form from a button click. trigger_id comes from # the click and expires after 3 seconds, so call this while answering it. - # The view comes from app/views/slack/code_of_conduct/form.slack_modal.slocks. + # The view comes from CodeOfConductSlackViews.form. # # The form has a required "I have read and accept" box, and says how Weave - # uses what it collects. It also asks for a first and last name when the - # user's name is missing (or when there is no linked user, since nothing is + # uses what it collects. It also asks for a preferred name, an optional + # legal name and an optional Slack nickname when the user's name is missing (or when there is no linked user, since nothing is # known about them). message is the DM the button was in, as # { channel:, ts: }. It goes into private_metadata, so the submission can # replace the DM. def open_code_of_conduct_form(trigger_id:, user:, message: nil) raise ConfigurationError, "Slack client not configured" unless @client - view = self.class.render_slack_view( - "form", :slack_modal, + view = CodeOfConductSlackViews.form( ask_name: user.nil? || user.name_missing?, coc_url: self.class.code_of_conduct_url, private_metadata: (message || {}).to_json @@ -218,15 +216,6 @@ def open_code_of_conduct_form(trigger_id:, user:, message: nil) @client.views_open(trigger_id: trigger_id, view: view) end - # Renders a Block Kit template from app/views/slack/code_of_conduct with - # slocks, and returns it as a Hash with symbol keys. - def self.render_slack_view(template, format, **assigns) - json = ApplicationController.render( - template: "slack/code_of_conduct/#{template}", formats: [format], layout: false, assigns: assigns - ) - JSON.parse(json, symbolize_names: true) - end - # Replaces a code-of-conduct DM with a thank-you, so its button can't be # used again. def mark_code_of_conduct_accepted(channel:, ts:, already_member: false) @@ -472,6 +461,24 @@ def configured? # Uses user token (requires users.profile:write scope) # Returns false when there is nothing to do (no Slack ID or no user token). # Slack errors are raised, so PushPronounsToSlackJob can retry them. + # Sets name fields on a Slack profile (first_name, last_name, + # display_name). Uses the user token (users.profile:write). + def update_slack_profile_name(slack_user_id, **profile) + return false if slack_user_id.blank? || profile.empty? + + unless @user_client + Rails.logger.warn "Slack user token not configured, cannot update the profile name" + return false + end + + @user_client.users_profile_set(user: slack_user_id, profile: profile.to_json) + Rails.logger.info "Updated Slack profile name (#{profile.keys.join(', ')}) for user #{slack_user_id}" + true + rescue Slack::Web::Api::Errors::SlackError, Slack::Web::Api::Errors::TooManyRequestsError => e + Rails.logger.error "Error updating Slack profile name for #{slack_user_id}: #{e.message}" + raise + end + def update_slack_pronouns(slack_user_id, pronouns) return false if slack_user_id.blank? diff --git a/app/views/slack/code_of_conduct/form.slack_modal.slocks b/app/views/slack/code_of_conduct/form.slack_modal.slocks deleted file mode 100644 index 38e3fca..0000000 --- a/app/views/slack/code_of_conduct/form.slack_modal.slocks +++ /dev/null @@ -1,30 +0,0 @@ -# The Slack form that accepts the code of conduct -# (SlackService#open_code_of_conduct_form). -# -# @ask_name true to show the name fields -# @coc_url the Code of Conduct, or nil -# @private_metadata JSON string - -title "Code of Conduct" -submit "I accept" -close "Not now" -callback SlackService::CODE_OF_CONDUCT_FORM -private_metadata @private_metadata - -if @coc_url.present? - section t("code_of_conduct_form.intro"), markdown: true, accessory: button("Read it", "read_coc", url: @coc_url) -else - section t("code_of_conduct_form.intro"), markdown: true -end - -if @ask_name - divider - section t("code_of_conduct_form.name_why"), markdown: true - input "First name", plain_text_input("value", max_length: 100), block_id: "first_name" - input "Last name", plain_text_input("value", max_length: 100), block_id: "last_name" -end - -divider -input "Code of Conduct", checkboxes("value", [option(t("code_of_conduct_form.accept_label"), "accept")]), block_id: "accept" - -context [mrkdwn_text(":lock: *#{t('code_of_conduct_form.data_use_title')}*\n#{t('code_of_conduct_form.data_use')}")] diff --git a/app/views/slack/code_of_conduct/request.slack_message.slocks b/app/views/slack/code_of_conduct/request.slack_message.slocks deleted file mode 100644 index e522bbe..0000000 --- a/app/views/slack/code_of_conduct/request.slack_message.slocks +++ /dev/null @@ -1,26 +0,0 @@ -# The Slack DM that asks someone to accept the code of conduct -# (SlackService#post_code_of_conduct). -# -# @title header text -# @paragraphs plain-text paragraphs (mrkdwn is allowed) -# @action_text what to do with the button -# @button { text:, action_id:, value: } -# @coc_url the Code of Conduct, or nil -# @web_url the /slack page, where people can also accept - -header @title - -@paragraphs.each { |paragraph| section paragraph, markdown: true } - -if @coc_url.present? - section ":scroll: *Code of Conduct*\nIt explains how we treat each other in Patchwork Labs.", - markdown: true, - accessory: button("Read it", "read_coc", url: @coc_url) -end - -divider - -section @action_text, markdown: true -actions [button(@button[:text], @button[:action_id], value: @button[:value], style: "primary")] - -context [mrkdwn_text("You can also accept on Weave: <#{@web_url}|#{@web_url.delete_prefix('https://')}>")] diff --git a/app/views/slack_onboarding/show.html.erb b/app/views/slack_onboarding/show.html.erb index c37cd96..ef10f24 100644 --- a/app/views/slack_onboarding/show.html.erb +++ b/app/views/slack_onboarding/show.html.erb @@ -86,12 +86,16 @@ <%= form_with url: accept_code_of_conduct_slack_onboarding_path, method: :post, class: "space-y-4 text-left" do |form| %> <% if current_user.name_missing? %>

<%= t("code_of_conduct_form.name_why") %>

- <% [[:first_name, "First name"], [:last_name, "Last name"]].each do |field, label| %> + <% CodeOfConductSlackViews::NAME_FIELDS.each do |field, options| %>
- <%= form.label field, label, class: "block text-sm font-medium text-ink" %> - <%= form.text_field field, value: params[field], required: true, maxlength: 100, - autocomplete: field == :first_name ? "given-name" : "family-name", + <%= form.label field, "#{options[:label]}#{' (optional)' if options[:optional]}", class: "block text-sm font-medium text-ink" %> + <%= form.text_field field, value: params[field], required: !options[:optional], maxlength: options.fetch(:max_length, 100), + autocomplete: { first_name: "given-name", last_name: "family-name", slack_name: "nickname" }.fetch(field, "off"), + aria: { describedby: "#{options.fetch(:hint_for, field)}_hint" }, class: "mt-1 w-full px-4 py-3 rounded-lg border-2 border-neutral-300 bg-paper text-ink focus:ring-2 focus:ring-grape focus:border-grape transition-colors" %> + <% if options[:hint] %> +

<%= t("code_of_conduct_form.#{options[:hint]}") %>

+ <% end %> <% if @form_errors&.key?(field) %>

<%= @form_errors[field] %>

<% end %> diff --git a/config/locales/code_of_conduct_form.en.yml b/config/locales/code_of_conduct_form.en.yml index d40b6c9..3f2689c 100644 --- a/config/locales/code_of_conduct_form.en.yml +++ b/config/locales/code_of_conduct_form.en.yml @@ -1,6 +1,6 @@ # The form people use to accept the code of conduct: the Slack form (see -# app/views/slack/code_of_conduct/form.slack_modal.slocks) and the /slack page. -# Both tell people why we ask for their data and how we use it. +# CodeOfConductSlackViews.form) and the /slack page. Both tell people why we +# ask for their data and how we use it. en: code_of_conduct_form: slack_action: "Choose the button below to accept." @@ -8,6 +8,9 @@ en: intro: "Read the Code of Conduct, then check the box below to accept it." accept_label: "I have read and accept the Code of Conduct" accept_error: "Check the box to accept the Code of Conduct." - name_why: "Your Slack profile has no name, so we don't know what to call you. Please add your first and last name." + name_why: "We don't have your name yet. Please tell us what to call you." + preferred_hint: "The name we call you. Weave and Patchwork Labs apps show it." + legal_hint: "Only if it is different from your preferred name. We use it only where the law asks for your legal name, for example a contract. Apps never get it." + slack_name_hint: "A nickname for Slack, if you don't want your preferred name there. Leave it blank to show your preferred name in Slack." data_use_title: "How we use this information" - data_use: "We save the date and time you accept in your Weave account, so we have a record that you accepted. If you add your name, we save it in your Weave account too. Weave admins can see this information. Patchwork Labs apps that you can use with Weave can see your name." + data_use: "We save the date and time you accept in your Weave account, so we have a record that you accepted. We save the names you add there too, and Weave admins can see them. Patchwork Labs apps that you can use with Weave can see your preferred name, but never your legal name. Your Slack profile shows your Slack nickname, or your preferred name if you leave the nickname blank." diff --git a/spec/jobs/push_slack_name_job_spec.rb b/spec/jobs/push_slack_name_job_spec.rb new file mode 100644 index 0000000..cc9b395 --- /dev/null +++ b/spec/jobs/push_slack_name_job_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe PushSlackNameJob do + let(:slack) { instance_double(SlackService) } + let!(:user) { create(:user, first_name: "Ada", last_name: "Lovelace", slack_id: "U1") } + + before { allow(SlackService).to receive(:new).and_return(slack) } + + it "sets only the Slack display name when there is a nickname" do + expect(slack).to receive(:update_slack_profile_name).with("U1", display_name: "ada") + + described_class.perform_now(user.id, nickname: "ada") + end + + it "sets the Slack first and last name to the preferred name otherwise" do + expect(slack).to receive(:update_slack_profile_name).with("U1", first_name: "Ada", last_name: "Lovelace") + + described_class.perform_now(user.id) + end + + it "does nothing for a user who is not in Slack" do + user.update!(slack_id: nil) + expect(slack).not_to receive(:update_slack_profile_name) + + described_class.perform_now(user.id) + end +end diff --git a/spec/requests/slack_interactions_spec.rb b/spec/requests/slack_interactions_spec.rb index 57733a7..5ad8d21 100644 --- a/spec/requests/slack_interactions_spec.rb +++ b/spec/requests/slack_interactions_spec.rb @@ -60,7 +60,7 @@ def signed_post(body) before { allow(SlackService).to receive(:new).and_return(slack) } - def submission(first_name:, last_name:, user_id: "U123", accept: true) + def submission(first_name:, last_name:, user_id: "U123", accept: true, slack_name: nil) { type: "view_submission", user: { id: user_id }, @@ -71,7 +71,10 @@ def submission(first_name:, last_name:, user_id: "U123", accept: true) values: { accept: { value: { selected_options: accept ? [{ value: "accept" }] : [] } }, first_name: { value: { value: first_name } }, - last_name: { value: { value: last_name } } + last_name: { value: { value: last_name } }, + legal_first_name: { value: { value: nil } }, + legal_last_name: { value: { value: nil } }, + slack_name: { value: { value: slack_name } } } } } @@ -105,10 +108,15 @@ def submission(first_name:, last_name:, user_id: "U123", accept: true) it "shows an error next to a missing name" do signed_post("payload=#{CGI.escape(submission(first_name: "Ada", last_name: ""))}") - expect(response.parsed_body).to eq("response_action" => "errors", "errors" => { "last_name" => "Enter your last name." }) + expect(response.parsed_body).to eq("response_action" => "errors", "errors" => { "last_name" => "Enter your preferred last name." }) expect(user.reload.slack_coc_accepted_at).to be_nil end + it "sets a Slack nickname instead of the preferred name when one is given" do + expect { signed_post("payload=#{CGI.escape(submission(first_name: "Ada", last_name: "Lovelace", slack_name: "ada"))}") } + .to have_enqueued_job(PushSlackNameJob).with(user.id, nickname: "ada") + end + it "does not accept without the box checked" do signed_post("payload=#{CGI.escape(submission(first_name: "Ada", last_name: "Lovelace", accept: false))}") diff --git a/spec/requests/slack_onboarding_spec.rb b/spec/requests/slack_onboarding_spec.rb index 9b7223f..d8e6e13 100644 --- a/spec/requests/slack_onboarding_spec.rb +++ b/spec/requests/slack_onboarding_spec.rb @@ -76,7 +76,7 @@ get slack_onboarding_path expect(response.body).to include("now accepts our Code of Conduct", "I have read and accept", "How we use this information") - expect(response.body).not_to include("First name") + expect(response.body).not_to include("Preferred first name") end it "asks for a name the Slack import could not find" do @@ -85,7 +85,7 @@ get slack_onboarding_path - expect(response.body).to include("First name", "Last name") + expect(response.body).to include("Preferred first name", "Legal first name (optional)", "Slack nickname (optional)") end end @@ -180,14 +180,34 @@ user.reload expect(user.full_name).to eq("Ada Lovelace") + expect(user.legal_name?).to be(false) expect(user.slack_coc_accepted_at).to be_present end + it "saves a legal name apart from the preferred name, and sends the preferred name to Slack" do + expect do + post accept_code_of_conduct_slack_onboarding_path, + params: { accept: "1", first_name: "Ada", last_name: "Lovelace", legal_first_name: "Augusta", legal_last_name: "King" } + end.to have_enqueued_job(PushSlackNameJob).with(user.id, nickname: nil) + + user.reload + expect(user.full_name).to eq("Ada Lovelace") + expect(user.legal_full_name).to eq("Augusta King") + end + + it "asks for both parts of a legal name" do + post accept_code_of_conduct_slack_onboarding_path, + params: { accept: "1", first_name: "Ada", last_name: "Lovelace", legal_first_name: "Augusta" } + + expect(response).to have_http_status(:unprocessable_content) + expect(response.body).to include("Enter both parts of your legal name") + end + it "does not accept without a name" do post accept_code_of_conduct_slack_onboarding_path, params: { accept: "1", first_name: "Ada", last_name: "" } expect(response).to have_http_status(:unprocessable_content) - expect(response.body).to include("Enter your last name.") + expect(response.body).to include("Enter your preferred last name.") expect(user.reload.slack_coc_accepted_at).to be_nil expect(user.first_name).to eq("NOTSET") end diff --git a/spec/services/slack_service_code_of_conduct_spec.rb b/spec/services/slack_service_code_of_conduct_spec.rb index fa4e72e..525fdb6 100644 --- a/spec/services/slack_service_code_of_conduct_spec.rb +++ b/spec/services/slack_service_code_of_conduct_spec.rb @@ -2,8 +2,8 @@ require "rails_helper" -# The code-of-conduct DM and form are Block Kit views rendered with slocks -# (app/views/slack/code_of_conduct). +# The code-of-conduct DM and form are Block Kit payloads built by +# CodeOfConductSlackViews. RSpec.describe SlackService do subject(:service) { described_class.new } @@ -49,7 +49,10 @@ def block_types(blocks) = blocks.map { |block| block[:type] } expect(args[:trigger_id]).to eq("T1") expect(view).to include(type: "modal", callback_id: "coc_form", private_metadata: { channel: "D1", ts: "1.2" }.to_json) inputs = view[:blocks].select { |block| block[:type] == "input" } - expect(inputs.map { |block| block[:block_id] }).to eq(%w[first_name last_name accept]) + expect(inputs.map { |block| [block[:block_id], block[:optional]] }).to eq( + [["first_name", false], ["last_name", false], ["legal_first_name", true], ["legal_last_name", true], + ["slack_name", true], ["accept", nil]] + ) expect(inputs.last[:element][:type]).to eq("checkboxes") expect(view[:blocks].last[:elements].first[:text]).to include("How we use this information") end