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