Address UX review
- Keep 'Deploy Section' open upon save, otherwise the token might get lost - When an error appears, display the error inside the form and also keep the Deploy Section open - Changue copy of revoke modal
This commit is contained in:
parent
370fc05da7
commit
345ac03b7a
|
@ -3,16 +3,12 @@ class Projects::DeployTokensController < Projects::ApplicationController
|
|||
|
||||
def create
|
||||
@token = DeployTokens::CreateService.new(@project, current_user, deploy_token_params).execute
|
||||
token_params = {}
|
||||
|
||||
if @token.valid?
|
||||
flash[:notice] = 'Your new project deploy token has been created.'
|
||||
else
|
||||
token_params = @token.attributes.slice("name", "scopes", "expires_at")
|
||||
flash[:alert] = @token.errors.full_messages.join(', ').html_safe
|
||||
end
|
||||
|
||||
redirect_to project_settings_repository_path(project, deploy_token: token_params)
|
||||
redirect_to project_settings_repository_path(project)
|
||||
end
|
||||
|
||||
def revoke
|
||||
|
|
|
@ -55,11 +55,9 @@ module Projects
|
|||
end
|
||||
|
||||
def define_deploy_token
|
||||
@deploy_token = @project.deploy_tokens.build(deploy_token_attributes)
|
||||
end
|
||||
|
||||
def deploy_token_attributes
|
||||
params.fetch(:deploy_token, {}).permit(:name, :expires_at, scopes: [])
|
||||
attributes = @deploy_tokens.attributes_deploy_token
|
||||
@deploy_token = @project.deploy_tokens.build(attributes)
|
||||
@deploy_token.valid? unless attributes.empty?
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -0,0 +1,7 @@
|
|||
module DeployTokensHelper
|
||||
def expand_deploy_tokens_section?(temporal_token, deploy_token)
|
||||
temporal_token.present? ||
|
||||
deploy_token.errors.present? ||
|
||||
Rails.env.test?
|
||||
end
|
||||
end
|
|
@ -19,8 +19,8 @@ class DeployToken < ActiveRecord::Base
|
|||
update!(revoked: true)
|
||||
end
|
||||
|
||||
def self.redis_shared_state_key(user_id)
|
||||
"gitlab:personal_access_token:#{user_id}"
|
||||
def redis_shared_state_key(user_id)
|
||||
"gitlab:deploy_token:#{project_id}:#{user_id}"
|
||||
end
|
||||
|
||||
def active?
|
||||
|
|
|
@ -23,14 +23,23 @@ module Projects
|
|||
end
|
||||
end
|
||||
|
||||
def new_deploy_token
|
||||
@new_deploy_token ||= Gitlab::Redis::SharedState.with do |redis|
|
||||
def temporal_token
|
||||
@temporal_token ||= Gitlab::Redis::SharedState.with do |redis|
|
||||
token = redis.get(deploy_token_key)
|
||||
redis.del(deploy_token_key)
|
||||
token
|
||||
end
|
||||
end
|
||||
|
||||
def attributes_deploy_token
|
||||
@attributes_deploy_token ||= Gitlab::Redis::SharedState.with do |redis|
|
||||
attributes_key = deploy_token_key + ":attributes"
|
||||
attributes_content = redis.get(attributes_key) || '{}'
|
||||
redis.del(attributes_key)
|
||||
JSON.parse(attributes_content)
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def scope_descriptions
|
||||
|
@ -41,7 +50,7 @@ module Projects
|
|||
end
|
||||
|
||||
def deploy_token_key
|
||||
DeployToken.redis_shared_state_key(current_user.id)
|
||||
@deploy_token_key ||= project.deploy_tokens.new.redis_shared_state_key(current_user.id)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -3,22 +3,36 @@ module DeployTokens
|
|||
REDIS_EXPIRY_TIME = 3.minutes
|
||||
|
||||
def execute
|
||||
@deploy_token = @project.deploy_tokens.create(params)
|
||||
store_in_redis if @deploy_token.persisted?
|
||||
|
||||
@deploy_token
|
||||
@project.deploy_tokens.build.tap do |deploy_token|
|
||||
deploy_token.attributes = params
|
||||
deploy_token.save
|
||||
store_deploy_token_info_in_redis(deploy_token)
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def store_in_redis
|
||||
Gitlab::Redis::SharedState.with do |redis|
|
||||
redis.set(deploy_token_key, @deploy_token.token, ex: REDIS_EXPIRY_TIME)
|
||||
def store_deploy_token_info_in_redis(deploy_token)
|
||||
deploy_token_key = deploy_token.redis_shared_state_key(current_user.id)
|
||||
|
||||
if deploy_token.persisted?
|
||||
store_in_redis(deploy_token_key, deploy_token.token)
|
||||
else
|
||||
store_deploy_attributes(deploy_token_key, deploy_token)
|
||||
end
|
||||
end
|
||||
|
||||
def deploy_token_key
|
||||
DeployToken.redis_shared_state_key(current_user.id)
|
||||
def store_deploy_attributes(deploy_token_key, deploy_token)
|
||||
attributes = deploy_token.attributes.slice("name", "expires_at")
|
||||
deploy_token_attributes_key = deploy_token_key + ":attributes"
|
||||
|
||||
store_in_redis(deploy_token_attributes_key, attributes.to_json)
|
||||
end
|
||||
|
||||
def store_in_redis(key, value)
|
||||
Gitlab::Redis::SharedState.with do |redis|
|
||||
redis.set(key, value, ex: REDIS_EXPIRY_TIME)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1,4 +1,4 @@
|
|||
- expanded = Rails.env.test?
|
||||
- expanded = expand_deploy_tokens_section?(@deploy_tokens.temporal_token, @deploy_token)
|
||||
|
||||
%section.settings.no-animate{ class: ('expanded' if expanded) }
|
||||
.settings-header
|
||||
|
@ -9,8 +9,8 @@
|
|||
%p
|
||||
Deploy tokens allow read-only access to your repository and registry images.
|
||||
.settings-content
|
||||
- if @deploy_tokens.new_deploy_token
|
||||
= render 'projects/deploy_tokens/new_deploy_token', new_token: @deploy_tokens.new_deploy_token
|
||||
- if @deploy_tokens.temporal_token
|
||||
= render 'projects/deploy_tokens/new_deploy_token', new_token: @deploy_tokens.temporal_token
|
||||
|
||||
%h5.prepend-top-0
|
||||
Add a deploy token
|
||||
|
|
|
@ -9,7 +9,9 @@
|
|||
%span{ "aria-hidden" => "true" } ×
|
||||
.modal-body
|
||||
%p
|
||||
Are you sure you want to revoke this Deploy Token? This action cannot be undone
|
||||
You are about to revoke
|
||||
%b #{token.name}.
|
||||
This action cannot be undone.
|
||||
.modal-footer
|
||||
%a{ href: '#', data: { dismiss: 'modal' }, class: 'btn btn-default' } Cancel
|
||||
= link_to "Revoke #{token.name}", revoke_project_deploy_token_path(project, token), method: :put, class: 'btn btn-danger'
|
||||
|
|
|
@ -27,6 +27,11 @@ describe Projects::DeployTokensController do
|
|||
subject
|
||||
expect(flash[:notice]).to eq('Your new project deploy token has been created.')
|
||||
end
|
||||
|
||||
it 'should redirect to project settings repository' do
|
||||
subject
|
||||
expect(response).to redirect_to project_settings_repository_path(project)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with invalid params' do
|
||||
|
@ -36,9 +41,9 @@ describe Projects::DeployTokensController do
|
|||
expect { subject }.not_to change(DeployToken, :count)
|
||||
end
|
||||
|
||||
it 'should include a flash alert with the error message' do
|
||||
it 'should redirect to project settings repository' do
|
||||
subject
|
||||
expect(flash[:alert]).to eq("Scopes can't be blank")
|
||||
expect(response).to redirect_to project_settings_repository_path(project)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -1,6 +1,6 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe Projects::Settings::RepositoryController do
|
||||
describe Projects::Settings::RepositoryController, :clean_gitlab_redis_shared_state do
|
||||
let(:project) { create(:project_empty_repo, :public) }
|
||||
let(:user) { create(:user) }
|
||||
|
||||
|
@ -17,7 +17,7 @@ describe Projects::Settings::RepositoryController do
|
|||
expect(response).to render_template(:show)
|
||||
end
|
||||
|
||||
context 'with no deploy token params' do
|
||||
context 'with no deploy token attributes present' do
|
||||
it 'should build an empty instance of DeployToken' do
|
||||
get :show, namespace_id: project.namespace, project_id: project
|
||||
|
||||
|
@ -29,17 +29,29 @@ describe Projects::Settings::RepositoryController do
|
|||
end
|
||||
end
|
||||
|
||||
context 'when rendering an invalid deploy token' do
|
||||
let(:deploy_token_attributes) { attributes_for(:deploy_token, project_id: project.id) }
|
||||
context 'with deploy token attributes present' do
|
||||
let(:deploy_token_key) { "gitlab:deploy_token:#{project.id}:#{user.id}:attributes" }
|
||||
|
||||
let(:deploy_token_attributes) do
|
||||
{
|
||||
name: 'test-token',
|
||||
expires_at: Date.today + 1.month
|
||||
}
|
||||
end
|
||||
|
||||
before do
|
||||
Gitlab::Redis::SharedState.with do |redis|
|
||||
redis.set(deploy_token_key, deploy_token_attributes.to_json)
|
||||
end
|
||||
|
||||
get :show, namespace_id: project.namespace, project_id: project
|
||||
end
|
||||
|
||||
it 'should build an instance of DeployToken' do
|
||||
get :show, namespace_id: project.namespace, project_id: project, deploy_token: deploy_token_attributes
|
||||
|
||||
deploy_token = assigns(:deploy_token)
|
||||
expect(deploy_token).to be_an_instance_of(DeployToken)
|
||||
expect(deploy_token.name).to eq(deploy_token_attributes[:name])
|
||||
expect(deploy_token.expires_at.to_date).to eq(deploy_token_attributes[:expires_at].to_date)
|
||||
expect(deploy_token.scopes).to match_array(deploy_token_attributes[:scopes])
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -90,17 +90,16 @@ feature 'Repository settings' do
|
|||
end
|
||||
|
||||
context 'Deploy tokens' do
|
||||
let(:deploy_token) { create(:deploy_token, project: project, expires_at: Date.today + 2.days) }
|
||||
let(:deploy_token) { create(:deploy_token, project: project) }
|
||||
|
||||
before do
|
||||
project.deploy_tokens << deploy_token
|
||||
visit project_settings_repository_path(project)
|
||||
end
|
||||
end
|
||||
|
||||
scenario 'view deploy tokens' do
|
||||
within('.deploy-tokens') do
|
||||
expect(page).to have_content(deploy_token.name)
|
||||
expect(page).to have_content('In 1 day')
|
||||
expect(page).to have_content(deploy_token.scopes.join(", "))
|
||||
end
|
||||
end
|
||||
|
@ -121,7 +120,6 @@ feature 'Repository settings' do
|
|||
click_link "Revoke #{deploy_token.name}"
|
||||
|
||||
expect(page).not_to have_content(deploy_token.name)
|
||||
expect(page).not_to have_content('In 1 day')
|
||||
expect(page).not_to have_content(deploy_token.scopes.join(", "))
|
||||
end
|
||||
end
|
||||
|
|
|
@ -6,40 +6,52 @@ describe DeployTokens::CreateService, :clean_gitlab_redis_shared_state do
|
|||
let(:deploy_token_params) { attributes_for(:deploy_token) }
|
||||
|
||||
describe '#execute' do
|
||||
subject { described_class.new(project, user, deploy_token_params) }
|
||||
subject { described_class.new(project, user, deploy_token_params).execute }
|
||||
|
||||
context 'when the deploy token is valid' do
|
||||
it 'should create a new DeployToken' do
|
||||
expect { subject.execute }.to change { DeployToken.count }.by(1)
|
||||
expect { subject }.to change { DeployToken.count }.by(1)
|
||||
end
|
||||
|
||||
it 'returns a DeployToken' do
|
||||
expect(subject).to be_an_instance_of DeployToken
|
||||
end
|
||||
|
||||
it 'should assign the DeployToken to the project' do
|
||||
subject.execute
|
||||
|
||||
expect(subject.project).to eq(project)
|
||||
end
|
||||
|
||||
it 'should store the token on redis' do
|
||||
subject.execute
|
||||
redis_key = DeployToken.redis_shared_state_key(user.id)
|
||||
redis_key = subject.redis_shared_state_key(user.id)
|
||||
|
||||
expect(Gitlab::Redis::SharedState.with { |redis| redis.get(redis_key) }).not_to be_nil
|
||||
end
|
||||
|
||||
it 'should not store deploy token attributes on redis' do
|
||||
redis_key = subject.redis_shared_state_key(user.id) + ":attributes"
|
||||
|
||||
expect(Gitlab::Redis::SharedState.with { |redis| redis.get(redis_key) }).to be_nil
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the deploy token is invalid' do
|
||||
let(:deploy_token_params) { attributes_for(:deploy_token, scopes: []) }
|
||||
|
||||
it 'it should not create a new DeployToken' do
|
||||
expect { subject.execute }.not_to change { DeployToken.count }
|
||||
expect { subject }.not_to change { DeployToken.count }
|
||||
end
|
||||
|
||||
it 'should not store the token on redis' do
|
||||
subject.execute
|
||||
redis_key = DeployToken.redis_shared_state_key(user.id)
|
||||
redis_key = subject.redis_shared_state_key(user.id)
|
||||
|
||||
expect(Gitlab::Redis::SharedState.with { |redis| redis.get(redis_key) }).to be_nil
|
||||
end
|
||||
|
||||
it 'should store deploy token attributes on redis' do
|
||||
redis_key = subject.redis_shared_state_key(user.id) + ":attributes"
|
||||
|
||||
expect(Gitlab::Redis::SharedState.with { |redis| redis.get(redis_key) }).not_to be_nil
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue