From 812bfb158b70b09cfd438379a4b9446aa85b52ec Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Fri, 13 Jul 2018 12:22:02 +0200 Subject: Add the `UserStatus` model This model will hold the status of a user, including these fields: - emoji: always present, with a default value - user: always present, foreign key to user - message: optional, maximum length of 100 The table also stores - cached_markdown_version - message_html For rendering markdown in the `message` field. --- app/models/user_status.rb | 13 +++++++++++++ db/migrate/20180713092803_create_user_statuses.rb | 20 ++++++++++++++++++++ db/schema.rb | 8 ++++++++ 3 files changed, 41 insertions(+) create mode 100644 app/models/user_status.rb create mode 100644 db/migrate/20180713092803_create_user_statuses.rb diff --git a/app/models/user_status.rb b/app/models/user_status.rb new file mode 100644 index 00000000000..df65d89b03c --- /dev/null +++ b/app/models/user_status.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class UserStatus < ActiveRecord::Base + include CacheMarkdownField + + belongs_to :user + + validates :user, presence: true + validates :emoji, inclusion: { in: Gitlab::Emoji.emojis_names } + validates :message, length: { maximum: 100 }, allow_blank: true + + cache_markdown_field :message, pipeline: :single_line +end diff --git a/db/migrate/20180713092803_create_user_statuses.rb b/db/migrate/20180713092803_create_user_statuses.rb new file mode 100644 index 00000000000..cbe21b89ad9 --- /dev/null +++ b/db/migrate/20180713092803_create_user_statuses.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class CreateUserStatuses < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + create_table :user_statuses, id: false, primary_key: :user_id do |t| + t.references :user, + foreign_key: { on_delete: :cascade }, + null: false, + primary_key: true + t.integer :cached_markdown_version, limit: 4 + t.string :emoji, null: false, default: 'speech_balloon' + t.string :message, limit: 100 + t.string :message_html + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 3db11d8447e..c91d565342e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -2048,6 +2048,13 @@ ActiveRecord::Schema.define(version: 20180722103201) do add_index "user_interacted_projects", ["project_id", "user_id"], name: "index_user_interacted_projects_on_project_id_and_user_id", unique: true, using: :btree add_index "user_interacted_projects", ["user_id"], name: "index_user_interacted_projects_on_user_id", using: :btree + create_table "user_statuses", primary_key: "user_id", force: :cascade do |t| + t.integer "cached_markdown_version" + t.string "emoji", default: "speech_balloon", null: false + t.string "message", limit: 100 + t.string "message_html" + end + create_table "user_synced_attributes_metadata", force: :cascade do |t| t.boolean "name_synced", default: false t.boolean "email_synced", default: false @@ -2349,6 +2356,7 @@ ActiveRecord::Schema.define(version: 20180722103201) do add_foreign_key "user_custom_attributes", "users", on_delete: :cascade add_foreign_key "user_interacted_projects", "projects", name: "fk_722ceba4f7", on_delete: :cascade add_foreign_key "user_interacted_projects", "users", name: "fk_0894651f08", on_delete: :cascade + add_foreign_key "user_statuses", "users", on_delete: :cascade add_foreign_key "user_synced_attributes_metadata", "users", on_delete: :cascade add_foreign_key "users", "application_setting_terms", column: "accepted_term_id", name: "fk_789cd90b35", on_delete: :cascade add_foreign_key "users_star_projects", "projects", name: "fk_22cd27ddfc", on_delete: :cascade -- cgit v1.2.1 From b4c4b48a8c0258ff266c523488aa169a1b5ea0f3 Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Fri, 13 Jul 2018 17:52:31 +0200 Subject: Allow users to set a status This can be done trough the API for the current user, or on the profile page. --- app/controllers/profiles_controller.rb | 3 +- app/models/user.rb | 2 + app/models/user_status.rb | 2 +- app/policies/user_policy.rb | 1 + app/services/users/set_status_service.rb | 38 +++++++++++++++ app/services/users/update_service.rb | 12 ++++- app/views/profiles/show.html.haml | 10 ++++ doc/api/users.md | 63 +++++++++++++++++++++++++ lib/api/entities.rb | 5 ++ lib/api/users.rb | 35 ++++++++++++++ lib/banzai/pipeline/emoji_pipeline.rb | 17 +++++++ locale/gitlab.pot | 8 +++- spec/controllers/profiles_controller_spec.rb | 9 ++++ spec/factories/user_statuses.rb | 9 ++++ spec/lib/banzai/pipeline/emoji_pipeline_spec.rb | 21 +++++++++ spec/models/user_spec.rb | 1 + spec/models/user_status_spec.rb | 20 ++++++++ spec/policies/user_policy_spec.rb | 4 ++ spec/requests/api/users_spec.rb | 62 ++++++++++++++++++++++++ spec/services/users/set_status_service_spec.rb | 58 +++++++++++++++++++++++ spec/services/users/update_service_spec.rb | 21 +++++++++ 21 files changed, 395 insertions(+), 6 deletions(-) create mode 100644 app/services/users/set_status_service.rb create mode 100644 lib/banzai/pipeline/emoji_pipeline.rb create mode 100644 spec/factories/user_statuses.rb create mode 100644 spec/lib/banzai/pipeline/emoji_pipeline_spec.rb create mode 100644 spec/models/user_status_spec.rb create mode 100644 spec/services/users/set_status_service_spec.rb diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb index 56a7b766b77..6f50cbb4a36 100644 --- a/app/controllers/profiles_controller.rb +++ b/app/controllers/profiles_controller.rb @@ -100,7 +100,8 @@ class ProfilesController < Profiles::ApplicationController :website_url, :organization, :preferred_language, - :private_profile + :private_profile, + status: [:emoji, :message] ) end end diff --git a/app/models/user.rb b/app/models/user.rb index 58429f8d607..a85a6d41378 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -141,6 +141,8 @@ class User < ActiveRecord::Base has_many :term_agreements belongs_to :accepted_term, class_name: 'ApplicationSetting::Term' + has_one :status, class_name: 'UserStatus' + # # Validations # diff --git a/app/models/user_status.rb b/app/models/user_status.rb index df65d89b03c..72938cfbca0 100644 --- a/app/models/user_status.rb +++ b/app/models/user_status.rb @@ -9,5 +9,5 @@ class UserStatus < ActiveRecord::Base validates :emoji, inclusion: { in: Gitlab::Emoji.emojis_names } validates :message, length: { maximum: 100 }, allow_blank: true - cache_markdown_field :message, pipeline: :single_line + cache_markdown_field :message, pipeline: :emoji end diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index b5717029354..e1efd84e510 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -16,6 +16,7 @@ class UserPolicy < BasePolicy rule { ~subject_ghost & (user_is_self | admin) }.policy do enable :destroy_user enable :update_user + enable :update_user_status end rule { default }.enable :read_user_profile diff --git a/app/services/users/set_status_service.rb b/app/services/users/set_status_service.rb new file mode 100644 index 00000000000..275e13faf59 --- /dev/null +++ b/app/services/users/set_status_service.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Users + class SetStatusService + include Gitlab::Allowable + + attr_reader :current_user, :target_user, :params + + def initialize(current_user, params) + @current_user, @params = current_user, params.dup + @target_user = params.delete(:user) || current_user + end + + def execute + return false unless can?(current_user, :update_user_status, target_user) + + if params[:emoji].present? || params[:message].present? + set_status + else + remove_status + end + end + + private + + def set_status + user_status.update(params) + end + + def remove_status + UserStatus.delete(target_user.id) + end + + def user_status + target_user.status || target_user.build_status + end + end +end diff --git a/app/services/users/update_service.rb b/app/services/users/update_service.rb index 6dadb5a4eac..a897e4bd56a 100644 --- a/app/services/users/update_service.rb +++ b/app/services/users/update_service.rb @@ -7,6 +7,7 @@ module Users def initialize(current_user, params = {}) @current_user = current_user @user = params.delete(:user) + @status_params = params.delete(:status) @params = params.dup end @@ -17,10 +18,11 @@ module Users assign_attributes(&block) - if @user.save(validate: validate) + if @user.save(validate: validate) && update_status notify_success(user_exists) else - error(@user.errors.full_messages.uniq.join('. ')) + messages = @user.errors.full_messages + Array(@user.status&.errors&.full_messages) + error(messages.uniq.join('. ')) end end @@ -34,6 +36,12 @@ module Users private + def update_status + return true unless @status_params + + Users::SetStatusService.new(current_user, @status_params.merge(user: @user)).execute + end + def notify_success(user_exists) notify_new_user(@user, nil) unless user_exists diff --git a/app/views/profiles/show.html.haml b/app/views/profiles/show.html.haml index a4835584b50..5e70cc09104 100644 --- a/app/views/profiles/show.html.haml +++ b/app/views/profiles/show.html.haml @@ -31,6 +31,16 @@ %hr = link_to _('Remove avatar'), profile_avatar_path, data: { confirm: _('Avatar will be removed. Are you sure?') }, method: :delete, class: 'btn btn-danger btn-inverted' %hr + .row + .col-lg-4.profile-settings-sidebar + %h4.prepend-top-0= s_("User|Current Status") + %p= _("This emoji and message will appear on your profile and throughout the interface.") + .col-lg-8 + .row + = f.fields_for :status, @user.status do |status_form| + = status_form.text_field :emoji + = status_form.text_field :message, maxlength: 100 + %hr .row .col-lg-4.profile-settings-sidebar %h4.prepend-top-0 diff --git a/doc/api/users.md b/doc/api/users.md index 07f9baf06d2..481219de0f1 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -440,6 +440,67 @@ GET /user } ``` +## User status + +Get the status of the currently signed in user. + +``` +GET /user/status +``` + +```json +{ + "emoji":"coffee", + "message":"I crave coffee" +} +``` + +## Get the status of a user + +Get the status of a user. + +``` +GET /users/:id_or_username/status +``` + +```json +{ + "emoji":"coffee", + "message":"I crave coffee" +} +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id_or_username` | string | yes | The id or username of the user to get a status of | + + +## Set user status + +Set the status of the current user. + +``` +PUT /user/status +``` + +```json +{ + "emoji":"coffee", + "message":"I crave coffee" +} +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `emoji` | string | no | The name of the emoji to use as status, if omitted `speech_balloon` is used. Emoji name can be one of the specified names in the [Gemojione index][gemojione-index]. | +| `message` | string | no | The message to set as a status | + +When both parameters are empty, the status will be cleared. + ## List user projects Please refer to the [List of user projects ](projects.md#list-user-projects). @@ -1167,3 +1228,5 @@ Example response: ``` Please note that `last_activity_at` is deprecated, please use `last_activity_on`. + +[gemojione-index]: https://github.com/jonathanwiesel/gemojione/blob/master/config/index.json diff --git a/lib/api/entities.rb b/lib/api/entities.rb index e883687f2db..f5a22e76efe 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -62,6 +62,11 @@ module API expose :admin?, as: :is_admin end + class UserStatus < Grape::Entity + expose :emoji + expose :message + end + class Email < Grape::Entity expose :id, :email end diff --git a/lib/api/users.rb b/lib/api/users.rb index e83887b3e9e..b0811bb4aad 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -121,6 +121,17 @@ module API present user, opts end + desc "Get the status of a user" + params do + requires :id_or_username, type: String, desc: 'The ID or username of the user' + end + get ":id_or_username/status" do + user = find_user(params[:id_or_username]) + not_found!('User') unless user && can?(current_user, :read_user, user) + + present user.status || {}, with: Entities::UserStatus + end + desc 'Create a user. Available only for admins.' do success Entities::UserPublic end @@ -740,6 +751,30 @@ module API present paginate(activities), with: Entities::UserActivity end + + desc 'Set the status of the current user' do + success Entities::UserStatus + end + params do + optional :emoji, type: String, desc: "The emoji to set on the status" + optional :message, type: String, desc: "The status message to set" + end + put "status" do + forbidden! unless can?(current_user, :update_user_status, current_user) + + if ::Users::SetStatusService.new(current_user, declared_params).execute + present current_user.status, with: Entities::UserStatus + else + render_validation_error!(current_user.status) + end + end + + desc 'get the status of the current user' do + success Entities::UserStatus + end + get 'status' do + present current_user.status || {}, with: Entities::UserStatus + end end end end diff --git a/lib/banzai/pipeline/emoji_pipeline.rb b/lib/banzai/pipeline/emoji_pipeline.rb new file mode 100644 index 00000000000..a1b522f4303 --- /dev/null +++ b/lib/banzai/pipeline/emoji_pipeline.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Banzai + module Pipeline + class EmojiPipeline < BasePipeline + # These filters will only perform sanitization of the content, preventing + # XSS, and replace emoji. + def self.filters + @filters ||= FilterArray[ + Filter::HtmlEntityFilter, + Filter::SanitizationFilter, + Filter::EmojiFilter + ] + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index a5bbe8938ff..60c1f18e9c7 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -8,8 +8,6 @@ msgid "" msgstr "" "Project-Id-Version: gitlab 1.0.0\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2018-07-10 16:02-0700\n" -"PO-Revision-Date: 2018-07-10 16:02-0700\n" "Last-Translator: FULL NAME \n" "Language-Team: LANGUAGE \n" "Language: \n" @@ -5149,6 +5147,9 @@ msgstr "" msgid "This directory" msgstr "" +msgid "This emoji and message will appear on your profile and throughout the interface." +msgstr "" + msgid "This group" msgstr "" @@ -5593,6 +5594,9 @@ msgstr "" msgid "Users" msgstr "" +msgid "User|Current Status" +msgstr "" + msgid "Variables" msgstr "" diff --git a/spec/controllers/profiles_controller_spec.rb b/spec/controllers/profiles_controller_spec.rb index 4530a301d4d..360c536c667 100644 --- a/spec/controllers/profiles_controller_spec.rb +++ b/spec/controllers/profiles_controller_spec.rb @@ -78,6 +78,15 @@ describe ProfilesController, :request_store do expect(ldap_user.name).not_to eq('John') expect(ldap_user.location).to eq('City, Country') end + + it 'allows setting a user status' do + sign_in(user) + + put :update, user: { status: { message: 'Working hard!' } } + + expect(user.reload.status.message).to eq('Working hard!') + expect(response).to have_gitlab_http_status(302) + end end describe 'PUT update_username' do diff --git a/spec/factories/user_statuses.rb b/spec/factories/user_statuses.rb new file mode 100644 index 00000000000..9998ae9609c --- /dev/null +++ b/spec/factories/user_statuses.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :user_status do + user + emoji 'coffee' + message 'I crave coffee' + end +end diff --git a/spec/lib/banzai/pipeline/emoji_pipeline_spec.rb b/spec/lib/banzai/pipeline/emoji_pipeline_spec.rb new file mode 100644 index 00000000000..744df3e0b96 --- /dev/null +++ b/spec/lib/banzai/pipeline/emoji_pipeline_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Banzai::Pipeline::EmojiPipeline do + def parse(text) + described_class.to_html(text, {}) + end + + it 'replaces emoji' do + expected_result = "Hello world #{Gitlab::Emoji.gl_emoji_tag('100')}" + + expect(parse('Hello world :100:')).to eq(expected_result) + end + + it 'filters out HTML tags' do + expected_result = "Hello <b>world</b> #{Gitlab::Emoji.gl_emoji_tag('100')}" + + expect(parse('Hello world :100:')).to eq(expected_result) + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index fc46551c3be..1afd6f234de 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -20,6 +20,7 @@ describe User do describe 'associations' do it { is_expected.to have_one(:namespace) } + it { is_expected.to have_one(:status) } it { is_expected.to have_many(:snippets).dependent(:destroy) } it { is_expected.to have_many(:members) } it { is_expected.to have_many(:project_members) } diff --git a/spec/models/user_status_spec.rb b/spec/models/user_status_spec.rb new file mode 100644 index 00000000000..fcc01cdae3d --- /dev/null +++ b/spec/models/user_status_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe UserStatus do + it { is_expected.to validate_presence_of(:user) } + + it { is_expected.to allow_value('smirk').for(:emoji) } + it { is_expected.not_to allow_value('hello world').for(:emoji) } + it { is_expected.not_to allow_value('').for(:emoji) } + + it { is_expected.to validate_length_of(:message).is_at_most(100) } + it { is_expected.to allow_value('').for(:message) } + + it 'is expected to be deleted when the user is deleted' do + status = create(:user_status) + + expect { status.user.destroy }.to change { described_class.count }.from(1).to(0) + end +end diff --git a/spec/policies/user_policy_spec.rb b/spec/policies/user_policy_spec.rb index a7a77abc3ee..7e0a1824200 100644 --- a/spec/policies/user_policy_spec.rb +++ b/spec/policies/user_policy_spec.rb @@ -35,6 +35,10 @@ describe UserPolicy do end end + describe "updating a user's status" do + it_behaves_like 'changing a user', :update_user_status + end + describe "destroying a user" do it_behaves_like 'changing a user', :destroy_user end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 6a051f865aa..da503897760 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -13,6 +13,26 @@ describe API::Users do let(:not_existing_pat_id) { (PersonalAccessToken.maximum('id') || 0 ) + 10 } let(:private_user) { create(:user, private_profile: true) } + shared_examples 'rendering user status' do + it 'returns the status if there was one' do + create(:user_status, user: user) + + get api(path, user) + + expect(response).to have_gitlab_http_status(:success) + expect(json_response['message']).to be_present + expect(json_response['emoji']).to be_present + end + + it 'returns an empty response if there was no status' do + get api(path, user) + + expect(response).to have_gitlab_http_status(:success) + expect(json_response['message']).to be_nil + expect(json_response['emoji']).to be_nil + end + end + describe 'GET /users' do context "when unauthenticated" do it "returns authorization error when the `username` parameter is not passed" do @@ -310,6 +330,20 @@ describe API::Users do end end + describe 'GET /users/:id_or_username/status' do + context 'when finding the user by id' do + it_behaves_like 'rendering user status' do + let(:path) { "/users/#{user.id}/status" } + end + end + + context 'when finding the user by username' do + it_behaves_like 'rendering user status' do + let(:path) { "/users/#{user.username}/status" } + end + end + end + describe "POST /users" do before do admin @@ -1774,6 +1808,34 @@ describe API::Users do end end + describe 'GET /user/status' do + let(:path) { '/user/status' } + it_behaves_like 'rendering user status' + end + + describe 'PUT /user/status' do + it 'saves the status' do + put api('/user/status', user), { emoji: 'smirk', message: 'hello world' } + + expect(response).to have_gitlab_http_status(:success) + expect(json_response['emoji']).to eq('smirk') + end + + it 'renders errors when the status was invalid' do + put api('/user/status', user), { emoji: 'does not exist', message: 'hello world' } + + expect(response).to have_gitlab_http_status(400) + expect(json_response['message']['emoji']).to be_present + end + + it 'deletes the status when passing empty values' do + put api('/user/status', user) + + expect(response).to have_gitlab_http_status(:success) + expect(user.reload.status).to be_nil + end + end + describe 'GET /users/:user_id/impersonation_tokens' do let!(:active_personal_access_token) { create(:personal_access_token, user: user) } let!(:revoked_personal_access_token) { create(:personal_access_token, :revoked, user: user) } diff --git a/spec/services/users/set_status_service_spec.rb b/spec/services/users/set_status_service_spec.rb new file mode 100644 index 00000000000..8a8458ab9de --- /dev/null +++ b/spec/services/users/set_status_service_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Users::SetStatusService do + let(:current_user) { create(:user) } + subject(:service) { described_class.new(current_user, params) } + + describe '#execute' do + context 'when when params are set' do + let(:params) { { emoji: 'taurus', message: 'a random status' } } + + it 'creates a status' do + service.execute + + expect(current_user.status.emoji).to eq('taurus') + expect(current_user.status.message).to eq('a random status') + end + + it 'updates a status if it already existed' do + create(:user_status, user: current_user) + + expect { service.execute }.not_to change { UserStatus.count } + expect(current_user.status.message).to eq('a random status') + end + + context 'for another user' do + let(:target_user) { create(:user) } + let(:params) do + { emoji: 'taurus', message: 'a random status', user: target_user } + end + + context 'the current user is admin' do + let(:current_user) { create(:admin) } + + it 'changes the status when the current user is allowed to do that' do + expect { service.execute }.to change { target_user.status } + end + end + + it 'does not update the status if the current user is not allowed' do + expect { service.execute }.not_to change { target_user.status } + end + end + end + + context 'without params' do + let(:params) { {} } + + it 'deletes the status' do + status = create(:user_status, user: current_user) + + expect { service.execute } + .to change { current_user.reload.status }.from(status).to(nil) + end + end + end +end diff --git a/spec/services/users/update_service_spec.rb b/spec/services/users/update_service_spec.rb index a4b7fe4674f..529c8485202 100644 --- a/spec/services/users/update_service_spec.rb +++ b/spec/services/users/update_service_spec.rb @@ -30,6 +30,27 @@ describe Users::UpdateService do expect(result[:message]).to eq('Username has already been taken') end + it 'updates the status if status params were given' do + update_user(user, status: { message: "On a call" }) + + expect(user.status.message).to eq("On a call") + end + + it 'does not delete the status if no status param was passed' do + create(:user_status, user: user, message: 'Busy!') + + update_user(user, name: 'New name') + + expect(user.status.message).to eq('Busy!') + end + + it 'includes status error messages' do + result = update_user(user, status: { emoji: "Moo!" }) + + expect(result[:status]).to eq(:error) + expect(result[:message]).to eq("Emoji is not included in the list") + end + def update_user(user, opts) described_class.new(user, opts.merge(user: user)).execute end -- cgit v1.2.1 From f1d3ea63cf74d2791a9a863b29ab2d919ea61bd0 Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Mon, 16 Jul 2018 18:18:52 +0200 Subject: Show the status of a user in interactions The status is shown for - The author of a commit when viewing a commit - Notes on a commit (regular/diff) - The user that triggered a pipeline when viewing a pipeline - The author of a merge request when viewing a merge request - The author of notes on a merge request (regular/diff) - The author of an issue when viewing an issue - The author of notes on an issue - The author of a snippet when viewing a snippet - The author of notes on a snippet - A user's profile page - The list of members of a group/user --- .../javascripts/notes/components/note_header.vue | 3 ++ .../vue_shared/components/header_ci_component.vue | 3 ++ app/controllers/concerns/members_presentation.rb | 8 +++++ app/controllers/concerns/membership_actions.rb | 2 ++ app/controllers/concerns/notes_actions.rb | 4 +-- app/controllers/concerns/renders_notes.rb | 5 +++ app/controllers/groups/group_members_controller.rb | 2 +- app/controllers/projects/commit_controller.rb | 9 +++-- app/controllers/projects/issues_controller.rb | 2 +- .../merge_requests/application_controller.rb | 2 +- app/controllers/projects/notes_controller.rb | 3 +- app/controllers/projects/pipelines_controller.rb | 6 +++- app/controllers/projects/snippets_controller.rb | 2 +- app/controllers/snippets/notes_controller.rb | 2 +- app/controllers/snippets_controller.rb | 2 +- app/helpers/issuables_helper.rb | 6 ++++ app/helpers/users_helper.rb | 18 ++++++++++ app/models/note.rb | 2 +- app/models/snippet.rb | 1 + app/models/user_status.rb | 4 +++ app/serializers/concerns/user_status_tooltip.rb | 19 ++++++++++ app/serializers/user_entity.rb | 1 + app/services/users/set_status_service.rb | 1 + app/views/projects/commit/_commit_box.html.haml | 1 + app/views/shared/members/_member.html.haml | 1 + app/views/shared/notes/_note.html.haml | 4 ++- app/views/shared/snippets/_header.html.haml | 1 + app/views/users/show.html.haml | 5 +++ .../unreleased/bvl-user-status-message-35463.yml | 5 +++ .../controllers/projects/issues_controller_spec.rb | 23 +++++++++++++ spec/features/groups/members/list_members_spec.rb | 14 +++++++- .../user_views_user_status_on_commit_spec.rb | 40 ++++++++++++++++++++++ .../projects/issues/user_views_issue_spec.rb | 18 ++++++++++ .../projects/members/group_members_spec.rb | 8 +++++ ...user_views_user_status_on_merge_request_spec.rb | 36 +++++++++++++++++++ spec/features/projects/pipelines/pipeline_spec.rb | 6 ++++ spec/features/projects/snippets/show_spec.rb | 12 +++++++ .../snippets/notes_on_personal_snippets_spec.rb | 11 ++++++ spec/features/snippets/show_spec.rb | 8 +++++ spec/features/users/show_spec.rb | 8 +++++ spec/support/matchers/user_status_matcher.rb | 13 +++++++ .../showing_user_status_shared_examples.rb | 11 ++++++ .../projects/merge_requests/show.html.haml_spec.rb | 11 ++++++ 43 files changed, 328 insertions(+), 15 deletions(-) create mode 100644 app/serializers/concerns/user_status_tooltip.rb create mode 100644 changelogs/unreleased/bvl-user-status-message-35463.yml create mode 100644 spec/features/projects/commit/user_views_user_status_on_commit_spec.rb create mode 100644 spec/features/projects/merge_requests/user_views_user_status_on_merge_request_spec.rb create mode 100644 spec/support/matchers/user_status_matcher.rb create mode 100644 spec/support/shared_examples/showing_user_status_shared_examples.rb diff --git a/app/assets/javascripts/notes/components/note_header.vue b/app/assets/javascripts/notes/components/note_header.vue index ee3580895df..a621418cf72 100644 --- a/app/assets/javascripts/notes/components/note_header.vue +++ b/app/assets/javascripts/notes/components/note_header.vue @@ -74,6 +74,9 @@ export default { {{ author.name }} + @{{ author.username }} diff --git a/app/assets/javascripts/vue_shared/components/header_ci_component.vue b/app/assets/javascripts/vue_shared/components/header_ci_component.vue index 62d35f6547d..49fbce75110 100644 --- a/app/assets/javascripts/vue_shared/components/header_ci_component.vue +++ b/app/assets/javascripts/vue_shared/components/header_ci_component.vue @@ -113,6 +113,9 @@ export default { {{ user.name }} + diff --git a/app/controllers/concerns/members_presentation.rb b/app/controllers/concerns/members_presentation.rb index c0622516fd3..215e0bdf3cb 100644 --- a/app/controllers/concerns/members_presentation.rb +++ b/app/controllers/concerns/members_presentation.rb @@ -2,10 +2,18 @@ module MembersPresentation extend ActiveSupport::Concern def present_members(members) + preload_associations(members) Gitlab::View::Presenter::Factory.new( members, current_user: current_user, presenter_class: MembersPresenter ).fabricate! end + + def preload_associations(members) + ActiveRecord::Associations::Preloader.new.preload(members, :user) + ActiveRecord::Associations::Preloader.new.preload(members, :source) + ActiveRecord::Associations::Preloader.new.preload(members.map(&:user), :status) + ActiveRecord::Associations::Preloader.new.preload(members.map(&:user), :u2f_registrations) + end end diff --git a/app/controllers/concerns/membership_actions.rb b/app/controllers/concerns/membership_actions.rb index 7a6a00b8e13..409e6d4c4d2 100644 --- a/app/controllers/concerns/membership_actions.rb +++ b/app/controllers/concerns/membership_actions.rb @@ -1,4 +1,5 @@ module MembershipActions + include MembersPresentation extend ActiveSupport::Concern def create @@ -20,6 +21,7 @@ module MembershipActions .execute(member) .present(current_user: current_user) + present_members([member]) respond_to do |format| format.js { render 'shared/members/update', locals: { member: member } } end diff --git a/app/controllers/concerns/notes_actions.rb b/app/controllers/concerns/notes_actions.rb index fe9a030cdf2..5127db3f5fb 100644 --- a/app/controllers/concerns/notes_actions.rb +++ b/app/controllers/concerns/notes_actions.rb @@ -41,7 +41,7 @@ module NotesActions @note = Notes::CreateService.new(note_project, current_user, create_params).execute if @note.is_a?(Note) - Notes::RenderService.new(current_user).execute([@note]) + prepare_notes_for_rendering([@note], noteable) end respond_to do |format| @@ -56,7 +56,7 @@ module NotesActions @note = Notes::UpdateService.new(project, current_user, note_params).execute(note) if @note.is_a?(Note) - Notes::RenderService.new(current_user).execute([@note]) + prepare_notes_for_rendering([@note]) end respond_to do |format| diff --git a/app/controllers/concerns/renders_notes.rb b/app/controllers/concerns/renders_notes.rb index 36e3d76ecfe..cf04023080a 100644 --- a/app/controllers/concerns/renders_notes.rb +++ b/app/controllers/concerns/renders_notes.rb @@ -4,6 +4,7 @@ module RendersNotes preload_noteable_for_regular_notes(notes) preload_max_access_for_authors(notes, @project) preload_first_time_contribution_for_authors(noteable, notes) + preload_author_status(notes) Notes::RenderService.new(current_user).execute(notes) notes @@ -28,4 +29,8 @@ module RendersNotes notes.each {|n| n.specialize_for_first_contribution!(noteable)} end + + def preload_author_status(notes) + ActiveRecord::Associations::Preloader.new.preload(notes, { author: :status }) + end end diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb index ef5d5e5c742..7dc51f4c357 100644 --- a/app/controllers/groups/group_members_controller.rb +++ b/app/controllers/groups/group_members_controller.rb @@ -30,7 +30,7 @@ class Groups::GroupMembersController < Groups::ApplicationController end @members = @members.page(params[:page]).per(50) - @members = present_members(@members.includes(:user)) + @members = present_members(@members) @requesters = present_members( AccessRequestsFinder.new(@group).execute(current_user)) diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 1d1184d46d1..44b176d304e 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -22,7 +22,9 @@ class Projects::CommitController < Projects::ApplicationController apply_diff_view_cookie! respond_to do |format| - format.html { render } + format.html do + render + end format.diff do send_git_diff(@project.repository, @commit.diff_refs) end @@ -124,7 +126,10 @@ class Projects::CommitController < Projects::ApplicationController end def commit - @noteable = @commit ||= @project.commit_by(oid: params[:id]) + @noteable = @commit ||= @project.commit_by(oid: params[:id]).tap do |commit| + # preload author and their status for rendering + commit&.author&.status + end end def define_commit_vars diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 7c897b2d86c..ef8159aa553 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -165,7 +165,7 @@ class Projects::IssuesController < Projects::ApplicationController return @issue if defined?(@issue) # The Sortable default scope causes performance issues when used with find_by - @issuable = @noteable = @issue ||= @project.issues.where(iid: params[:id]).reorder(nil).take! + @issuable = @noteable = @issue ||= @project.issues.includes(author: :status).where(iid: params[:id]).reorder(nil).take! @note = @project.notes.new(noteable: @issuable) return render_404 unless can?(current_user, :read_issue, @issue) diff --git a/app/controllers/projects/merge_requests/application_controller.rb b/app/controllers/projects/merge_requests/application_controller.rb index 8e4aeec16dc..fead81dd472 100644 --- a/app/controllers/projects/merge_requests/application_controller.rb +++ b/app/controllers/projects/merge_requests/application_controller.rb @@ -6,7 +6,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont private def merge_request - @issuable = @merge_request ||= @project.merge_requests.find_by!(iid: params[:id]) + @issuable = @merge_request ||= @project.merge_requests.includes(author: :status).find_by!(iid: params[:id]) end def merge_request_params diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index 4d4c2af2415..21e2145b73b 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -1,4 +1,5 @@ class Projects::NotesController < Projects::ApplicationController + include RendersNotes include NotesActions include NotesHelper include ToggleAwardEmoji @@ -53,7 +54,7 @@ class Projects::NotesController < Projects::ApplicationController private def render_json_with_notes_serializer - Notes::RenderService.new(current_user).execute([note]) + prepare_notes_for_rendering([note]) render json: note_serializer.represent(note) end diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 45cef123c34..1d5d1ee5d5a 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -161,7 +161,11 @@ class Projects::PipelinesController < Projects::ApplicationController end def pipeline - @pipeline ||= project.pipelines.find_by!(id: params[:id]).present(current_user: current_user) + @pipeline ||= project + .pipelines + .includes(user: :status) + .find_by!(id: params[:id]) + .present(current_user: current_user) end def commit diff --git a/app/controllers/projects/snippets_controller.rb b/app/controllers/projects/snippets_controller.rb index f742d7edf83..7c03d8ce827 100644 --- a/app/controllers/projects/snippets_controller.rb +++ b/app/controllers/projects/snippets_controller.rb @@ -88,7 +88,7 @@ class Projects::SnippetsController < Projects::ApplicationController protected def snippet - @snippet ||= @project.snippets.find(params[:id]) + @snippet ||= @project.snippets.inc_relations_for_view.find(params[:id]) end alias_method :awardable, :snippet alias_method :spammable, :snippet diff --git a/app/controllers/snippets/notes_controller.rb b/app/controllers/snippets/notes_controller.rb index c8b4682e6dc..217da89a1fd 100644 --- a/app/controllers/snippets/notes_controller.rb +++ b/app/controllers/snippets/notes_controller.rb @@ -9,7 +9,7 @@ class Snippets::NotesController < ApplicationController private def note - @note ||= snippet.notes.find(params[:id]) + @note ||= snippet.notes.inc_relations_for_view.find(params[:id]) end alias_method :awardable, :note diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb index 1d6d0943674..dcf18c1f751 100644 --- a/app/controllers/snippets_controller.rb +++ b/app/controllers/snippets_controller.rb @@ -95,7 +95,7 @@ class SnippetsController < ApplicationController protected def snippet - @snippet ||= PersonalSnippet.find_by(id: params[:id]) + @snippet ||= PersonalSnippet.inc_relations_for_view.find_by(id: params[:id]) end alias_method :awardable, :snippet diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 8766bb43cac..678fed9c414 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -159,6 +159,12 @@ module IssuablesHelper output << content_tag(:strong) do author_output = link_to_member(project, issuable.author, size: 24, mobile_classes: "d-none d-sm-inline", tooltip: true) author_output << link_to_member(project, issuable.author, size: 24, by_username: true, avatar: false, mobile_classes: "d-block d-sm-none") + + if status = user_status(issuable.author) + author_output << "  #{status}".html_safe + end + + author_output end output << " ".html_safe diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb index 8ee4203b6f5..ceea4384f91 100644 --- a/app/helpers/users_helper.rb +++ b/app/helpers/users_helper.rb @@ -39,6 +39,24 @@ module UsersHelper "access:#{max_project_member_access(project)}" end + def user_status(user) + return unless user + + unless user.association(:status).loaded? + exception = RuntimeError.new("Status was not preloaded") + Gitlab::Sentry.track_exception(exception, extra: { user: user.inspect }) + end + + return unless user.status + + content_tag :span, + class: 'user-status-emoji has-tooltip', + title: user.status.message_html, + data: { html: true, placement: 'top' } do + emoji_icon user.status.emoji + end + end + private def get_profile_tabs diff --git a/app/models/note.rb b/app/models/note.rb index fe3507adcb3..87cf7c7b2fa 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -101,7 +101,7 @@ class Note < ActiveRecord::Base scope :inc_author_project, -> { includes(:project, :author) } scope :inc_author, -> { includes(:author) } scope :inc_relations_for_view, -> do - includes(:project, :author, :updated_by, :resolved_by, :award_emoji, + includes(:project, { author: :status }, :updated_by, :resolved_by, :award_emoji, :system_note_metadata, :note_diff_file) end diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 644120453cf..390bdbf838a 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -49,6 +49,7 @@ class Snippet < ActiveRecord::Base scope :are_public, -> { where(visibility_level: Snippet::PUBLIC) } scope :public_and_internal, -> { where(visibility_level: [Snippet::PUBLIC, Snippet::INTERNAL]) } scope :fresh, -> { order("created_at DESC") } + scope :inc_relations_for_view, -> { includes(author: :status) } participant :author participant :notes_with_associations diff --git a/app/models/user_status.rb b/app/models/user_status.rb index 72938cfbca0..2bbb0c59ac1 100644 --- a/app/models/user_status.rb +++ b/app/models/user_status.rb @@ -3,6 +3,10 @@ class UserStatus < ActiveRecord::Base include CacheMarkdownField + self.primary_key = :user_id + + DEFAULT_EMOJI = 'speech_balloon'.freeze + belongs_to :user validates :user, presence: true diff --git a/app/serializers/concerns/user_status_tooltip.rb b/app/serializers/concerns/user_status_tooltip.rb new file mode 100644 index 00000000000..aa6e67e3351 --- /dev/null +++ b/app/serializers/concerns/user_status_tooltip.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module UserStatusTooltip + extend ActiveSupport::Concern + include ActionView::Helpers::TagHelper + include ActionView::Context + include EmojiHelper + include UsersHelper + + included do + expose :user_status_if_loaded, as: :status_tooltip_html + + def user_status_if_loaded + return nil unless object.association(:status).loaded? + + user_status(object) + end + end +end diff --git a/app/serializers/user_entity.rb b/app/serializers/user_entity.rb index 6236d66ff4a..656900bb8af 100644 --- a/app/serializers/user_entity.rb +++ b/app/serializers/user_entity.rb @@ -2,6 +2,7 @@ class UserEntity < API::Entities::UserBasic include RequestAwareEntity + include UserStatusTooltip expose :path do |user| user_path(user) diff --git a/app/services/users/set_status_service.rb b/app/services/users/set_status_service.rb index 275e13faf59..89008368d5f 100644 --- a/app/services/users/set_status_service.rb +++ b/app/services/users/set_status_service.rb @@ -24,6 +24,7 @@ module Users private def set_status + params[:emoji] = UserStatus::DEFAULT_EMOJI if params[:emoji].blank? user_status.update(params) end diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index 78522393d4b..8afbbaf378f 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -13,6 +13,7 @@ = author_avatar(@commit, size: 24, has_tooltip: false) %strong = commit_author_link(@commit, avatar: true, size: 24) + = user_status(@commit.author) - if @commit.different_committer? %span.light= _('Committed by') %strong diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml index 46debe1f2b9..af29c0fe59e 100644 --- a/app/views/shared/members/_member.html.haml +++ b/app/views/shared/members/_member.html.haml @@ -11,6 +11,7 @@ = image_tag avatar_icon_for_user(user, 40), class: "avatar s40", alt: '' .user-info = link_to user.name, user_path(user), class: 'member' + = user_status(user) %span.cgray= user.to_reference - if user == current_user diff --git a/app/views/shared/notes/_note.html.haml b/app/views/shared/notes/_note.html.haml index f5464058bc0..84adbd444c5 100644 --- a/app/views/shared/notes/_note.html.haml +++ b/app/views/shared/notes/_note.html.haml @@ -31,7 +31,9 @@ .note-header .note-header-info %a{ href: user_path(note.author) } - %span.note-header-author-name= sanitize(note.author.name) + %span.note-header-author-name + = sanitize(note.author.name) + = user_status(note.author) %span.note-headline-light = note.author.to_reference %span.note-headline-light diff --git a/app/views/shared/snippets/_header.html.haml b/app/views/shared/snippets/_header.html.haml index 828ec870dc0..10bfc30492a 100644 --- a/app/views/shared/snippets/_header.html.haml +++ b/app/views/shared/snippets/_header.html.haml @@ -8,6 +8,7 @@ Authored = time_ago_with_tooltip(@snippet.created_at, placement: 'bottom', html_class: 'snippet_updated_ago') by #{link_to_member(@project, @snippet.author, size: 24, author_class: "author item-title", avatar_class: "d-none d-sm-inline")} + = user_status(@snippet.author) .detail-page-header-actions - if @snippet.project_id? diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml index 8d9e86d02c4..7a38d290915 100644 --- a/app/views/users/show.html.haml +++ b/app/views/users/show.html.haml @@ -40,6 +40,11 @@ .cover-title = @user.name + - if @user.status + .cover-status + = emoji_icon(@user.status.emoji) + = markdown_field(@user.status, :message) + .cover-desc.member-date %p %span.middle-dot-divider diff --git a/changelogs/unreleased/bvl-user-status-message-35463.yml b/changelogs/unreleased/bvl-user-status-message-35463.yml new file mode 100644 index 00000000000..981f0608c29 --- /dev/null +++ b/changelogs/unreleased/bvl-user-status-message-35463.yml @@ -0,0 +1,5 @@ +--- +title: Users can set a status message and emoji +merge_request: 20614 +author: +type: added diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index ff1835a34c2..5b347b1109a 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -993,6 +993,29 @@ describe Projects::IssuesController do expect(json_response.first.keys).to match_array(%w[id reply_id expanded notes diff_discussion discussion_path individual_note resolvable resolved resolved_at resolved_by resolved_by_push commit_id for_commit project_id]) end + it 'renders the author status html if there is a status' do + create(:user_status, user: discussion.author) + + get :discussions, namespace_id: project.namespace, project_id: project, id: issue.iid + + note_json = json_response.first['notes'].first + + expect(note_json['author']['status_tooltip_html']).to be_present + end + + it 'does not cause an extra query for the status' do + control = ActiveRecord::QueryRecorder.new do + get :discussions, namespace_id: project.namespace, project_id: project, id: issue.iid + end + + create(:user_status, user: discussion.author) + second_discussion = create(:discussion_note_on_issue, noteable: issue, project: issue.project, author: create(:user)) + create(:user_status, user: second_discussion.author) + + expect { get :discussions, namespace_id: project.namespace, project_id: project, id: issue.iid } + .not_to exceed_query_limit(control) + end + context 'with cross-reference system note', :request_store do let(:new_issue) { create(:issue) } let(:cross_reference) { "mentioned in #{new_issue.to_reference(issue.project)}" } diff --git a/spec/features/groups/members/list_members_spec.rb b/spec/features/groups/members/list_members_spec.rb index 33f93fcc470..e1587a8b6a5 100644 --- a/spec/features/groups/members/list_members_spec.rb +++ b/spec/features/groups/members/list_members_spec.rb @@ -9,7 +9,7 @@ describe 'Groups > Members > List members' do let(:nested_group) { create(:group, parent: group) } before do - gitlab_sign_in(user1) + sign_in(user1) end it 'show members from current group and parent', :nested_groups do @@ -32,6 +32,18 @@ describe 'Groups > Members > List members' do expect(second_row).to be_blank end + describe 'showing status of members' do + before do + group.add_developer(user2) + end + + subject { visit group_group_members_path(group) } + + it_behaves_like 'showing user status' do + let(:user_with_status) { user2 } + end + end + def first_row page.all('ul.content-list > li')[0] end diff --git a/spec/features/projects/commit/user_views_user_status_on_commit_spec.rb b/spec/features/projects/commit/user_views_user_status_on_commit_spec.rb new file mode 100644 index 00000000000..e78b7f7ae08 --- /dev/null +++ b/spec/features/projects/commit/user_views_user_status_on_commit_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Project > Commit > View user status' do + include RepoHelpers + + set(:project) { create(:project, :repository) } + set(:user) { create(:user) } + let(:commit_author) { create(:user, email: sample_commit.author_email) } + + before do + sign_in(user) + project.add_developer(user) + end + + subject { visit(project_commit_path(project, sample_commit.id)) } + + describe 'status for the commit author' do + it_behaves_like 'showing user status' do + let(:user_with_status) { commit_author } + end + end + + describe 'status for a comment on the commit' do + let(:note) { create(:note, :on_commit, project: project) } + + it_behaves_like 'showing user status' do + let(:user_with_status) { note.author } + end + end + + describe 'status for a diff note on the commit' do + let(:note) { create(:diff_note_on_commit, project: project) } + + it_behaves_like 'showing user status' do + let(:user_with_status) { note.author } + end + end +end diff --git a/spec/features/projects/issues/user_views_issue_spec.rb b/spec/features/projects/issues/user_views_issue_spec.rb index 4093876c289..117e5986f29 100644 --- a/spec/features/projects/issues/user_views_issue_spec.rb +++ b/spec/features/projects/issues/user_views_issue_spec.rb @@ -29,4 +29,22 @@ describe "User views issue" do expect(page).not_to have_link('Close issue') end end + + describe 'user status' do + subject { visit(project_issue_path(project, issue)) } + + describe 'showing status of the author of the issue' do + it_behaves_like 'showing user status' do + let(:user_with_status) { issue.author } + end + end + + describe 'showing status of a user who commented on an issue', :js do + let!(:note) { create(:note, noteable: issue, project: project, author: user_with_status) } + + it_behaves_like 'showing user status' do + let(:user_with_status) { create(:user) } + end + end + end end diff --git a/spec/features/projects/members/group_members_spec.rb b/spec/features/projects/members/group_members_spec.rb index 41b2beb40b9..0b2cd13b8ec 100644 --- a/spec/features/projects/members/group_members_spec.rb +++ b/spec/features/projects/members/group_members_spec.rb @@ -87,4 +87,12 @@ describe 'Projects members' do end end end + + describe 'showing status of members' do + it_behaves_like 'showing user status' do + let(:user_with_status) { developer } + + subject { visit project_settings_members_path(project) } + end + end end diff --git a/spec/features/projects/merge_requests/user_views_user_status_on_merge_request_spec.rb b/spec/features/projects/merge_requests/user_views_user_status_on_merge_request_spec.rb new file mode 100644 index 00000000000..78d9c6c6db1 --- /dev/null +++ b/spec/features/projects/merge_requests/user_views_user_status_on_merge_request_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Project > Merge request > View user status' do + let(:project) { create(:project, :public, :repository) } + let(:merge_request) do + create(:merge_request, source_project: project, target_project: project, author: create(:user)) + end + + subject { visit merge_request_path(merge_request) } + + describe 'the status of the merge request author' do + it_behaves_like 'showing user status' do + let(:user_with_status) { merge_request.author } + end + end + + context 'for notes', :js do + describe 'the status of the author of a note on a merge request' do + let(:note) { create(:note, noteable: merge_request, project: project, author: create(:user)) } + + it_behaves_like 'showing user status' do + let(:user_with_status) { note.author } + end + end + + describe 'the status of the author of a diff note on a merge request' do + let(:note) { create(:diff_note_on_merge_request, noteable: merge_request, project: project, author: create(:user)) } + + it_behaves_like 'showing user status' do + let(:user_with_status) { note.author } + end + end + end +end diff --git a/spec/features/projects/pipelines/pipeline_spec.rb b/spec/features/projects/pipelines/pipeline_spec.rb index ecc7cf84138..a84492ea5f1 100644 --- a/spec/features/projects/pipelines/pipeline_spec.rb +++ b/spec/features/projects/pipelines/pipeline_spec.rb @@ -63,6 +63,12 @@ describe 'Pipeline', :js do expect(page).to have_css('#js-tab-pipeline.active') end + it_behaves_like 'showing user status' do + let(:user_with_status) { pipeline.user } + + subject { visit project_pipeline_path(project, pipeline) } + end + describe 'pipeline graph' do context 'when pipeline has running builds' do it 'shows a running icon and a cancel action for the running build' do diff --git a/spec/features/projects/snippets/show_spec.rb b/spec/features/projects/snippets/show_spec.rb index 3cc797277dd..f3dc13fb52f 100644 --- a/spec/features/projects/snippets/show_spec.rb +++ b/spec/features/projects/snippets/show_spec.rb @@ -141,4 +141,16 @@ describe 'Projects > Snippets > Project snippet', :js do end end end + + it_behaves_like 'showing user status' do + let(:file_name) { 'ruby-style-guide.md' } + let(:content) { project.repository.blob_at('master', 'files/markdown/ruby-style-guide.md').data } + + let(:user_with_status) { snippet.author } + + subject do + visit project_snippet_path(project, snippet) + wait_for_requests + end + end end diff --git a/spec/features/snippets/notes_on_personal_snippets_spec.rb b/spec/features/snippets/notes_on_personal_snippets_spec.rb index 269351e55c9..1442e011d52 100644 --- a/spec/features/snippets/notes_on_personal_snippets_spec.rb +++ b/spec/features/snippets/notes_on_personal_snippets_spec.rb @@ -16,6 +16,8 @@ describe 'Comments on personal snippets', :js do before do sign_in user visit snippet_path(snippet) + + wait_for_requests end subject { page } @@ -42,6 +44,15 @@ describe 'Comments on personal snippets', :js do expect(page).to have_selector('.note-emoji-button') end end + + it 'shows the status of a note author' do + status = create(:user_status, user: user) + visit snippet_path(snippet) + + within("#note_#{snippet_notes[0].id}") do + expect(page).to show_user_status(status) + end + end end context 'when submitting a note' do diff --git a/spec/features/snippets/show_spec.rb b/spec/features/snippets/show_spec.rb index f31457db92f..3fe0b60b18f 100644 --- a/spec/features/snippets/show_spec.rb +++ b/spec/features/snippets/show_spec.rb @@ -155,4 +155,12 @@ describe 'Snippet', :js do end end end + + it_behaves_like 'showing user status' do + let(:file_name) { 'popen.rb' } + let(:content) { project.repository.blob_at('master', 'files/ruby/popen.rb').data } + let(:user_with_status) { snippet.author } + + subject { visit snippet_path(snippet) } + end end diff --git a/spec/features/users/show_spec.rb b/spec/features/users/show_spec.rb index 207c333c636..bc07ab48c39 100644 --- a/spec/features/users/show_spec.rb +++ b/spec/features/users/show_spec.rb @@ -53,6 +53,14 @@ describe 'User page' do end end + it 'shows the status if there was one' do + create(:user_status, user: user, message: "Working hard!") + + visit(user_path(user)) + + expect(page).to have_content("Working hard!") + end + context 'signup disabled' do it 'shows the sign in link' do stub_application_setting(signup_enabled: false) diff --git a/spec/support/matchers/user_status_matcher.rb b/spec/support/matchers/user_status_matcher.rb new file mode 100644 index 00000000000..3cf240d874a --- /dev/null +++ b/spec/support/matchers/user_status_matcher.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +RSpec::Matchers.define :show_user_status do |status| + match do |page| + expect(page).to have_selector(".user-status-emoji[title='#{status.message}']") + + # The same user status might be displayed multiple times on the page + emoji_span = page.first(".user-status-emoji[title='#{status.message}']") + page.within(emoji_span) do + expect(page).to have_emoji(status.emoji) + end + end +end diff --git a/spec/support/shared_examples/showing_user_status_shared_examples.rb b/spec/support/shared_examples/showing_user_status_shared_examples.rb new file mode 100644 index 00000000000..eef769de2fc --- /dev/null +++ b/spec/support/shared_examples/showing_user_status_shared_examples.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +shared_examples 'showing user status' do + let!(:status) { create(:user_status, user: user_with_status, emoji: 'smirk', message: 'Authoring this object') } + + it 'shows the status' do + subject + + expect(page).to show_user_status(status) + end +end diff --git a/spec/views/projects/merge_requests/show.html.haml_spec.rb b/spec/views/projects/merge_requests/show.html.haml_spec.rb index fe6ad26a6f6..fa6c4ce4ac8 100644 --- a/spec/views/projects/merge_requests/show.html.haml_spec.rb +++ b/spec/views/projects/merge_requests/show.html.haml_spec.rb @@ -17,6 +17,13 @@ describe 'projects/merge_requests/show.html.haml' do author: user) end + def preload_view_requirements + # This will load the status fields of the author of the note and merge request + # to avoid queries in when rendering the view being tested. + closed_merge_request.author.status + note.author.status + end + before do assign(:project, project) assign(:merge_request, closed_merge_request) @@ -26,6 +33,8 @@ describe 'projects/merge_requests/show.html.haml' do assign(:notes, []) assign(:pipelines, Ci::Pipeline.none) + preload_view_requirements + allow(view).to receive_messages(current_user: user, can?: true, current_application_settings: Gitlab::CurrentSettings.current_application_settings) @@ -42,6 +51,7 @@ describe 'projects/merge_requests/show.html.haml' do it 'does not show the "Reopen" button when the source project does not exist' do unlink_project.execute closed_merge_request.reload + preload_view_requirements render @@ -56,6 +66,7 @@ describe 'projects/merge_requests/show.html.haml' do forked_project.destroy # Reload merge request so MergeRequest#source_project turns to `nil` closed_merge_request.reload + preload_view_requirements render -- cgit v1.2.1 From 9252414078c61a9642889d19fd166cf658492ef2 Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Tue, 24 Jul 2018 16:16:09 +0200 Subject: Hide the status fields behind a feature flag Since the frontend for this feature isn't ready, better to hide the confusing field behind a feature flag. --- app/helpers/profiles_helper.rb | 4 ++++ app/views/profiles/show.html.haml | 22 ++++++++++-------- .../unreleased/bvl-user-status-message-35463.yml | 2 +- spec/features/profiles/user_edit_profile_spec.rb | 27 ++++++++++++++++++++++ 4 files changed, 44 insertions(+), 11 deletions(-) diff --git a/app/helpers/profiles_helper.rb b/app/helpers/profiles_helper.rb index e7aa92e6e5c..a6a57db3002 100644 --- a/app/helpers/profiles_helper.rb +++ b/app/helpers/profiles_helper.rb @@ -9,4 +9,8 @@ module ProfilesHelper end end end + + def show_user_status_field? + Feature.enabled?(:user_status_form) || cookies[:feature_user_status_form] == 'true' + end end diff --git a/app/views/profiles/show.html.haml b/app/views/profiles/show.html.haml index 5e70cc09104..73099297a38 100644 --- a/app/views/profiles/show.html.haml +++ b/app/views/profiles/show.html.haml @@ -30,16 +30,18 @@ - if @user.avatar? %hr = link_to _('Remove avatar'), profile_avatar_path, data: { confirm: _('Avatar will be removed. Are you sure?') }, method: :delete, class: 'btn btn-danger btn-inverted' - %hr - .row - .col-lg-4.profile-settings-sidebar - %h4.prepend-top-0= s_("User|Current Status") - %p= _("This emoji and message will appear on your profile and throughout the interface.") - .col-lg-8 - .row - = f.fields_for :status, @user.status do |status_form| - = status_form.text_field :emoji - = status_form.text_field :message, maxlength: 100 + + - if show_user_status_field? + %hr + .row + .col-lg-4.profile-settings-sidebar + %h4.prepend-top-0= s_("User|Current Status") + %p= _("This emoji and message will appear on your profile and throughout the interface.") + .col-lg-8 + .row + = f.fields_for :status, @user.status do |status_form| + = status_form.text_field :emoji + = status_form.text_field :message, maxlength: 100 %hr .row .col-lg-4.profile-settings-sidebar diff --git a/changelogs/unreleased/bvl-user-status-message-35463.yml b/changelogs/unreleased/bvl-user-status-message-35463.yml index 981f0608c29..c844e7ea0e4 100644 --- a/changelogs/unreleased/bvl-user-status-message-35463.yml +++ b/changelogs/unreleased/bvl-user-status-message-35463.yml @@ -1,5 +1,5 @@ --- title: Users can set a status message and emoji merge_request: 20614 -author: +author: niedermyer & davamr type: added diff --git a/spec/features/profiles/user_edit_profile_spec.rb b/spec/features/profiles/user_edit_profile_spec.rb index 0b5eacbe916..96bbe6f93f1 100644 --- a/spec/features/profiles/user_edit_profile_spec.rb +++ b/spec/features/profiles/user_edit_profile_spec.rb @@ -55,4 +55,31 @@ describe 'User edit profile' do expect(page).to have_link('gravatar.com') end end + + context 'user status' do + it 'hides user status when the feature is disabled' do + stub_feature_flags(user_status_form: false) + + visit(profile_path) + + expect(page).not_to have_content('Current Status') + end + + it 'shows the status form when the feature is enabled' do + stub_feature_flags(user_status_form: true) + + visit(profile_path) + + expect(page).to have_content('Current Status') + end + + it 'shows the status form when the feature is enabled by setting a cookie', :js do + stub_feature_flags(user_status_form: false) + set_cookie('feature_user_status_form', 'true') + + visit(profile_path) + + expect(page).to have_content('Current Status') + end + end end -- cgit v1.2.1 From 229558bca0b29e00bd4f08e72e2b707d899e78ce Mon Sep 17 00:00:00 2001 From: Winnie Hellmann Date: Thu, 26 Jul 2018 09:48:58 +0000 Subject: Make it explicit that the status message can contain emojis --- doc/api/users.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/users.md b/doc/api/users.md index 481219de0f1..8290279c295 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -497,7 +497,7 @@ Parameters: | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | | `emoji` | string | no | The name of the emoji to use as status, if omitted `speech_balloon` is used. Emoji name can be one of the specified names in the [Gemojione index][gemojione-index]. | -| `message` | string | no | The message to set as a status | +| `message` | string | no | The message to set as a status. It can also contain emoji codes. | When both parameters are empty, the status will be cleared. -- cgit v1.2.1 From 003dfc422ec8290921504e458339a89f955d8f4c Mon Sep 17 00:00:00 2001 From: Winnie Hellmann Date: Thu, 26 Jul 2018 09:51:43 +0000 Subject: Make it explicit that the message can contain emojis in the settings --- app/views/profiles/show.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/profiles/show.html.haml b/app/views/profiles/show.html.haml index 73099297a38..e7044f722c5 100644 --- a/app/views/profiles/show.html.haml +++ b/app/views/profiles/show.html.haml @@ -36,7 +36,7 @@ .row .col-lg-4.profile-settings-sidebar %h4.prepend-top-0= s_("User|Current Status") - %p= _("This emoji and message will appear on your profile and throughout the interface.") + %p= s_("Profiles|This emoji and message will appear on your profile and throughout the interface. The message can contain emoji codes, too.") .col-lg-8 .row = f.fields_for :status, @user.status do |status_form| -- cgit v1.2.1 From 78c6f9a2d6d3211402ca07813871c5b4f1a1dc57 Mon Sep 17 00:00:00 2001 From: Winnie Hellmann Date: Thu, 26 Jul 2018 12:29:04 +0200 Subject: Regenerate locale/gitlab.pot --- locale/gitlab.pot | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 60c1f18e9c7..c88c5e5e98b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4049,6 +4049,9 @@ msgstr "" msgid "Profiles|This doesn't look like a public SSH key, are you sure you want to add it?" msgstr "" +msgid "Profiles|This emoji and message will appear on your profile and throughout the interface. The message can contain emoji codes, too." +msgstr "" + msgid "Profiles|Type your %{confirmationValue} to confirm:" msgstr "" @@ -5147,9 +5150,6 @@ msgstr "" msgid "This directory" msgstr "" -msgid "This emoji and message will appear on your profile and throughout the interface." -msgstr "" - msgid "This group" msgstr "" -- cgit v1.2.1 From 12095251c3777c5231cab97854d5dca69d31cc5d Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Thu, 26 Jul 2018 17:14:02 +0200 Subject: Add the message HTML to the UserStatus api --- doc/api/users.md | 54 ++++++++++++++++++++++++++--------------- lib/api/entities.rb | 3 +++ spec/requests/api/users_spec.rb | 1 + 3 files changed, 39 insertions(+), 19 deletions(-) diff --git a/doc/api/users.md b/doc/api/users.md index 8290279c295..a8858468cab 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -448,10 +448,17 @@ Get the status of the currently signed in user. GET /user/status ``` +```bash +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/user/status" +``` + +Example response: + ```json { "emoji":"coffee", - "message":"I crave coffee" + "message":"I crave coffee :coffee:", + "message_html": "I crave coffee ☕" } ``` @@ -463,20 +470,24 @@ Get the status of a user. GET /users/:id_or_username/status ``` +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `id_or_username` | string | yes | The id or username of the user to get a status of | + +```bash +curl "https://gitlab.example.com/users/janedoe/status" +``` + +Example response: + ```json { "emoji":"coffee", - "message":"I crave coffee" + "message":"I crave coffee :coffee:", + "message_html": "I crave coffee ☕" } ``` -Parameters: - -| Attribute | Type | Required | Description | -| --------- | ---- | -------- | ----------- | -| `id_or_username` | string | yes | The id or username of the user to get a status of | - - ## Set user status Set the status of the current user. @@ -485,21 +496,26 @@ Set the status of the current user. PUT /user/status ``` -```json -{ - "emoji":"coffee", - "message":"I crave coffee" -} -``` - -Parameters: - | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | | `emoji` | string | no | The name of the emoji to use as status, if omitted `speech_balloon` is used. Emoji name can be one of the specified names in the [Gemojione index][gemojione-index]. | | `message` | string | no | The message to set as a status. It can also contain emoji codes. | -When both parameters are empty, the status will be cleared. +When both parameters `emoji` and `message` are empty, the status will be cleared. + +```bash +curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" --data "emoji=coffee" --data "emoji=I crave coffee" https://gitlab.example.com/api/v4/user/status +``` + +Example responses + +```json +{ + "emoji":"coffee", + "message":"I crave coffee", + "message_html": "I crave coffee" +} +``` ## List user projects diff --git a/lib/api/entities.rb b/lib/api/entities.rb index f5a22e76efe..efd148c1a7f 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -65,6 +65,9 @@ module API class UserStatus < Grape::Entity expose :emoji expose :message + expose :message_html do |entity| + MarkupHelper.markdown_field(entity, :message) + end end class Email < Grape::Entity diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index da503897760..d48d577afa1 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -21,6 +21,7 @@ describe API::Users do expect(response).to have_gitlab_http_status(:success) expect(json_response['message']).to be_present + expect(json_response['message_html']).to be_present expect(json_response['emoji']).to be_present end -- cgit v1.2.1