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..b05d6cf 100644 --- a/app/controllers/slack_onboarding_controller.rb +++ b/app/controllers/slack_onboarding_controller.rb @@ -42,16 +42,44 @@ 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." + # "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, + names: params.slice(*CodeOfConductAcceptance::NAME_FIELDS).permit(*CodeOfConductAcceptance::NAME_FIELDS).to_h.symbolize_keys + ) + unless result.success? + render_form_errors(result.errors) + 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 + + 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 ccb8e3e..388f0a2 100644 --- a/app/controllers/webhooks/slack_controller.rb +++ b/app/controllers/webhooks/slack_controller.rb @@ -57,26 +57,42 @@ 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 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. 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 +100,50 @@ 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) + + # 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, + names: CodeOfConductAcceptance::NAME_FIELDS.index_with { |field| values.dig(field.to_s, "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..d824652 --- /dev/null +++ b/app/jobs/code_of_conduct_request_job.rb @@ -0,0 +1,60 @@ +# 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? + + 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 + +end 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/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..84e204f --- /dev/null +++ b/app/services/code_of_conduct_acceptance.rb @@ -0,0 +1,76 @@ +# 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 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 + + 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, names:, message:) + @user = user + @names = NAME_FIELDS.index_with { |field| names[field].to_s.strip } + @message = message + end + + def call + if @user.name_missing? + errors = name_errors + return Result.new(user: @user, errors:) if errors.any? + + save_names + 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 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 + + 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 ffc2556..cdd7825 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 = CodeOfConductSlackViews::FORM_CALLBACK + # How long a workspace's label -> field ID map is cached. CUSTOM_FIELD_IDS_TTL = 1.hour @@ -151,39 +154,84 @@ 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. The + # 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. + # + # 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." + 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", value: slack_user_id } + end + + 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"), + 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 to get full access.", - blocks: [ - { 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", - value: slack_user_id - } - ] - } - ] + text: "Please review and accept the Patchwork Labs Code of Conduct.", + 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 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 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 = CodeOfConductSlackViews.form( + 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: view) + 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 @@ -413,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/services/slack_webhook_service.rb b/app/services/slack_webhook_service.rb index 14ffc63..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) + 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/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..ef10f24 100644 --- a/app/views/slack_onboarding/show.html.erb +++ b/app/views/slack_onboarding/show.html.erb @@ -66,18 +66,58 @@ <%= 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? %> +

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

+ <% CodeOfConductSlackViews::NAME_FIELDS.each do |field, options| %> +
+ <%= 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 %> +
+ <% 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/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_form.en.yml b/config/locales/code_of_conduct_form.en.yml new file mode 100644 index 0000000..3f2689c --- /dev/null +++ b/config/locales/code_of_conduct_form.en.yml @@ -0,0 +1,16 @@ +# The form people use to accept the code of conduct: the Slack form (see +# 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." + 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: "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. 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/config/locales/code_of_conduct_request.en.yml b/config/locales/code_of_conduct_request.en.yml new file mode 100644 index 0000000..3a23c26 --- /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_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/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..0eed6a8 --- /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", 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) + + 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", 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", 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 + + 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/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/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..5ad8d21 100644 --- a/spec/requests/slack_interactions_spec.rb +++ b/spec/requests/slack_interactions_spec.rb @@ -45,6 +45,95 @@ 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", accept: true, slack_name: nil) + { + type: "view_submission", + user: { id: user_id }, + view: { + callback_id: "coc_form", + private_metadata: { channel: "D1", ts: "1.2" }.to_json, + state: { + values: { + accept: { value: { selected_options: accept ? [{ value: "accept" }] : [] } }, + first_name: { value: { value: first_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 } } + } + } + } + }.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 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))}") + + 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") + + 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..d8e6e13 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 have read and accept", "How we use this information") + expect(response.body).not_to include("Preferred 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("Preferred first name", "Legal first name (optional)", "Slack nickname (optional)") + end end describe "POST /slack" do @@ -112,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 @@ -123,15 +142,76 @@ .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 - 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, params: { accept: "1" } + + 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: { accept: "1", first_name: "Ada", last_name: "Lovelace" } + + 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 preferred 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 +224,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_service_code_of_conduct_spec.rb b/spec/services/slack_service_code_of_conduct_spec.rb new file mode 100644 index 0000000..525fdb6 --- /dev/null +++ b/spec/services/slack_service_code_of_conduct_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require "rails_helper" + +# The code-of-conduct DM and form are Block Kit payloads built by +# CodeOfConductSlackViews. +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], 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 + + 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 aefe8f6..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") + 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, 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