From 597c5c762ff5c86f8fc8c2c3c3aed2af270c2a71 Mon Sep 17 00:00:00 2001 From: "Luke \"Jared\" Bennett" Date: Wed, 26 Apr 2017 11:17:53 +0100 Subject: [PATCH] Fixed being behind --- README.md | 2 +- app/assets/stylesheets/framework/blocks.scss | 1 - app/assets/stylesheets/framework/nav.scss | 2 +- .../users/migrate_to_ghost_user_service.rb | 36 +++++--- ...01-add-slash-slack-commands-to-api-doc.yml | 5 ++ ...tion-while-moving-issues-to-ghost-user.yml | 4 + changelogs/unreleased/fix_link_in_readme.yml | 4 + doc/administration/integration/plantuml.md | 2 +- doc/api/services.md | 83 ++++++++++++++++--- doc/development/fe_guide/testing.md | 6 +- .../migrate_to_ghost_user_service_spec.rb | 18 ++++ ...e_to_ghost_user_service_shared_examples.rb | 52 ++++++++++++ 12 files changed, 184 insertions(+), 31 deletions(-) create mode 100644 changelogs/unreleased/29801-add-slash-slack-commands-to-api-doc.yml create mode 100644 changelogs/unreleased/30306-transaction-while-moving-issues-to-ghost-user.yml create mode 100644 changelogs/unreleased/fix_link_in_readme.yml diff --git a/README.md b/README.md index f0e3b52ef6f..10d69efdc6b 100644 --- a/README.md +++ b/README.md @@ -73,7 +73,7 @@ One small thing you also have to do when installing it yourself is to copy the e cp config/unicorn.rb.example.development config/unicorn.rb -Instructions on how to start GitLab and how to run the tests can be found in the [development section of the GitLab Development Kit](https://gitlab.com/gitlab-org/gitlab-development-kit#development). +Instructions on how to start GitLab and how to run the tests can be found in the [getting started section of the GitLab Development Kit](https://gitlab.com/gitlab-org/gitlab-development-kit#getting-started). ## Software stack diff --git a/app/assets/stylesheets/framework/blocks.scss b/app/assets/stylesheets/framework/blocks.scss index 52425262925..f3e2a5db0a6 100644 --- a/app/assets/stylesheets/framework/blocks.scss +++ b/app/assets/stylesheets/framework/blocks.scss @@ -230,7 +230,6 @@ float: right; margin-top: 8px; padding-bottom: 8px; - border-bottom: 1px solid $border-color; } } diff --git a/app/assets/stylesheets/framework/nav.scss b/app/assets/stylesheets/framework/nav.scss index e6d808717f3..b6cf5101d60 100644 --- a/app/assets/stylesheets/framework/nav.scss +++ b/app/assets/stylesheets/framework/nav.scss @@ -110,7 +110,7 @@ .top-area { @include clearfix; - border-bottom: 1px solid $white-normal; + border-bottom: 1px solid $border-color; .nav-text { padding-top: 16px; diff --git a/app/services/users/migrate_to_ghost_user_service.rb b/app/services/users/migrate_to_ghost_user_service.rb index 1e1ed1791ec..4628c4c6f6e 100644 --- a/app/services/users/migrate_to_ghost_user_service.rb +++ b/app/services/users/migrate_to_ghost_user_service.rb @@ -15,20 +15,20 @@ module Users end def execute - # Block the user before moving records to prevent a data race. - # For example, if the user creates an issue after `migrate_issues` - # runs and before the user is destroyed, the destroy will fail with - # an exception. - user.block + transition = user.block_transition user.transaction do - @ghost_user = User.ghost + # Block the user before moving records to prevent a data race. + # For example, if the user creates an issue after `migrate_issues` + # runs and before the user is destroyed, the destroy will fail with + # an exception. + user.block - migrate_issues - migrate_merge_requests - migrate_notes - migrate_abuse_reports - migrate_award_emoji + # Reverse the user block if record migration fails + if !migrate_records && transition + transition.rollback + user.save! + end end user.reload @@ -36,6 +36,18 @@ module Users private + def migrate_records + user.transaction(requires_new: true) do + @ghost_user = User.ghost + + migrate_issues + migrate_merge_requests + migrate_notes + migrate_abuse_reports + migrate_award_emojis + end + end + def migrate_issues user.issues.update_all(author_id: ghost_user.id) end @@ -52,7 +64,7 @@ module Users user.reported_abuse_reports.update_all(reporter_id: ghost_user.id) end - def migrate_award_emoji + def migrate_award_emojis user.award_emoji.update_all(user_id: ghost_user.id) end end diff --git a/changelogs/unreleased/29801-add-slash-slack-commands-to-api-doc.yml b/changelogs/unreleased/29801-add-slash-slack-commands-to-api-doc.yml new file mode 100644 index 00000000000..9c5df690085 --- /dev/null +++ b/changelogs/unreleased/29801-add-slash-slack-commands-to-api-doc.yml @@ -0,0 +1,5 @@ +--- +title: Add Slack slash command api to services documentation and rearrange order and + cases +merge_request: 10757 +author: TM Lee diff --git a/changelogs/unreleased/30306-transaction-while-moving-issues-to-ghost-user.yml b/changelogs/unreleased/30306-transaction-while-moving-issues-to-ghost-user.yml new file mode 100644 index 00000000000..5fc57e44be6 --- /dev/null +++ b/changelogs/unreleased/30306-transaction-while-moving-issues-to-ghost-user.yml @@ -0,0 +1,4 @@ +--- +title: Add a transaction around move_issues_to_ghost_user +merge_request: 10465 +author: diff --git a/changelogs/unreleased/fix_link_in_readme.yml b/changelogs/unreleased/fix_link_in_readme.yml new file mode 100644 index 00000000000..be5ceac8656 --- /dev/null +++ b/changelogs/unreleased/fix_link_in_readme.yml @@ -0,0 +1,4 @@ +--- +title: Fix dead link to GDK on the README page +merge_request: +author: Dino Maric diff --git a/doc/administration/integration/plantuml.md b/doc/administration/integration/plantuml.md index 5c856835039..b21817c1fd3 100644 --- a/doc/administration/integration/plantuml.md +++ b/doc/administration/integration/plantuml.md @@ -28,7 +28,7 @@ using Tomcat: sudo apt-get install tomcat7 sudo cp target/plantuml.war /var/lib/tomcat7/webapps/plantuml.war sudo chown tomcat7:tomcat7 /var/lib/tomcat7/webapps/plantuml.war -sudo service restart tomcat7 +sudo service tomcat7 restart ``` Once the Tomcat service restarts the PlantUML service will be ready and diff --git a/doc/api/services.md b/doc/api/services.md index 7d4779f1137..0f42c256099 100644 --- a/doc/api/services.md +++ b/doc/api/services.md @@ -490,13 +490,78 @@ Remove all previously JIRA settings from a project. DELETE /projects/:id/services/jira ``` -## Mattermost Slash Commands +## Slack slash commands + +Ability to receive slash commands from a Slack chat instance. + +### Get Slack slash command service settings + +Get Slack slash command service settings for a project. + +``` +GET /projects/:id/services/slack-slash-commands +``` + +Example response: + +```json +{ + "id": 4, + "title": "Slack slash commands", + "created_at": "2017-06-27T05:51:39-07:00", + "updated_at": "2017-06-27T05:51:39-07:00", + "active": true, + "push_events": true, + "issues_events": true, + "merge_requests_events": true, + "tag_push_events": true, + "note_events": true, + "build_events": true, + "pipeline_events": true, + "properties": { + "token": "9koXpg98eAheJpvBs5tK" + } +} +``` + +### Create/Edit Slack slash command service + +Set Slack slash command for a project. + +``` +PUT /projects/:id/services/slack-slash-commands +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------- | ---- | -------- | ----------- | +| `token` | string | yes | The Slack token | + + +### Delete Slack slash command service + +Delete Slack slash command service for a project. + +``` +DELETE /projects/:id/services/slack-slash-commands +``` + +## Mattermost slash commands Ability to receive slash commands from a Mattermost chat instance. -### Create/Edit Mattermost Slash Command service +### Get Mattermost slash command service settings -Set Mattermost Slash Command for a project. +Get Mattermost slash command service settings for a project. + +``` +GET /projects/:id/services/mattermost-slash-commands +``` + +### Create/Edit Mattermost slash command service + +Set Mattermost slash command for a project. ``` PUT /projects/:id/services/mattermost-slash-commands @@ -509,22 +574,14 @@ Parameters: | `token` | string | yes | The Mattermost token | -### Delete Mattermost Slash Command service +### Delete Mattermost slash command service -Delete Mattermost Slash Command service for a project. +Delete Mattermost slash command service for a project. ``` DELETE /projects/:id/services/mattermost-slash-commands ``` -### Get Mattermost Slash Command service settings - -Get Mattermost Slash Command service settings for a project. - -``` -GET /projects/:id/services/mattermost-slash-commands -``` - ## Pipeline-Emails Get emails for GitLab CI pipelines. diff --git a/doc/development/fe_guide/testing.md b/doc/development/fe_guide/testing.md index 66afbf4db4d..157c13352ca 100644 --- a/doc/development/fe_guide/testing.md +++ b/doc/development/fe_guide/testing.md @@ -14,8 +14,10 @@ for more information on general testing practices at GitLab. GitLab uses the [Karma][karma] test runner with [Jasmine][jasmine] as its test framework for our JavaScript unit tests. For tests that rely on DOM -manipulation we use fixtures which are pre-compiled from HAML source files and -served during testing by the [jasmine-jquery][jasmine-jquery] plugin. +manipulation, we generate HTML files using RSpec suites (see `spec/javascripts/fixtures/*.rb` for examples). +Some fixtures are still HAML templates that are translated to HTML files using the same mechanism (see `static_fixtures.rb`). +Those will be migrated over time. +Fixtures are served during testing by the [jasmine-jquery][jasmine-jquery] plugin. JavaScript tests live in `spec/javascripts/`, matching the folder structure of `app/assets/javascripts/`: `app/assets/javascripts/behaviors/autosize.js` diff --git a/spec/services/users/migrate_to_ghost_user_service_spec.rb b/spec/services/users/migrate_to_ghost_user_service_spec.rb index 8c5b7e41c15..9e1edf1ac30 100644 --- a/spec/services/users/migrate_to_ghost_user_service_spec.rb +++ b/spec/services/users/migrate_to_ghost_user_service_spec.rb @@ -60,5 +60,23 @@ describe Users::MigrateToGhostUserService, services: true do end end end + + context "when record migration fails with a rollback exception" do + before do + expect_any_instance_of(MergeRequest::ActiveRecord_Associations_CollectionProxy) + .to receive(:update_all).and_raise(ActiveRecord::Rollback) + end + + context "for records that were already migrated" do + let!(:issue) { create(:issue, project: project, author: user) } + let!(:merge_request) { create(:merge_request, source_project: project, author: user, target_branch: "first") } + + it "reverses the migration" do + service.execute + + expect(issue.reload.author).to eq(user) + end + end + end end end diff --git a/spec/support/services/migrate_to_ghost_user_service_shared_examples.rb b/spec/support/services/migrate_to_ghost_user_service_shared_examples.rb index 0eac587e973..dcc562c684b 100644 --- a/spec/support/services/migrate_to_ghost_user_service_shared_examples.rb +++ b/spec/support/services/migrate_to_ghost_user_service_shared_examples.rb @@ -35,5 +35,57 @@ shared_examples "migrating a deleted user's associated records to the ghost user expect(user).to be_blocked end + + context "race conditions" do + context "when #{record_class_name} migration fails and is rolled back" do + before do + expect_any_instance_of(record_class::ActiveRecord_Associations_CollectionProxy) + .to receive(:update_all).and_raise(ActiveRecord::Rollback) + end + + it 'rolls back the user block' do + service.execute + + expect(user.reload).not_to be_blocked + end + + it "doesn't unblock an previously-blocked user" do + user.block + + service.execute + + expect(user.reload).to be_blocked + end + end + + context "when #{record_class_name} migration fails with a non-rollback exception" do + before do + expect_any_instance_of(record_class::ActiveRecord_Associations_CollectionProxy) + .to receive(:update_all).and_raise(ArgumentError) + end + + it 'rolls back the user block' do + service.execute rescue nil + + expect(user.reload).not_to be_blocked + end + + it "doesn't unblock an previously-blocked user" do + user.block + + service.execute rescue nil + + expect(user.reload).to be_blocked + end + end + + it "blocks the user before #{record_class_name} migration begins" do + expect(service).to receive("migrate_#{record_class_name.parameterize('_')}s".to_sym) do + expect(user.reload).to be_blocked + end + + service.execute + end + end end end