Merge pull request #12104 from rubhanazeem/token-show

Simplify token creation code

src/api/app/components/copy_to_clipboard_input_component.html.haml

@@ -1,5 +1,4 @@
--# TODO: Relying on a form builder is a bit weird. We could perhaps directly use `text_field_tag` instead and pass an input value to the component.
-= @form_builder.text_field(:string_readonly, id: 'copy-to-clipboard-readonly', class: 'form-control', readonly: true)
+= text_field_tag(nil, @token_string, readonly: true, id: 'copy-to-clipboard-readonly', class: 'form-control')
 .input-group-append#copy-to-clipboard
   %span.input-group-text
     %i.fas.fa-clipboard

src/api/app/components/copy_to_clipboard_input_component.rb

@@ -1,6 +1,6 @@
 class CopyToClipboardInputComponent < ApplicationComponent
-  def initialize(form_builder:)
+  def initialize(token_string:)
     super
-    @form_builder = form_builder
+    @token_string = token_string
   end
 end

src/api/app/components/token_card_component.html.haml

@@ -1,10 +1,11 @@
 .card.m-1.p-2
   .card-body
-    - if @token.name.present?
-      %h5.card-title
-        = @token.name
-    - else
-      %h5.card-title.font-italic No name
+    = link_to token_path(@token) do
+      - if @token.name.present?
+        %h5.card-title
+          = @token.name
+      - else
+        %h5.card-title.font-italic No name
     %p.card-text
       Id: #{@token.id}
     %p.card-text
@@ -21,7 +22,7 @@
         - if policy(@token).edit?
           = link_to(edit_token_path(@token), title: 'Edit Token') do
             %i.fas.fa-edit
-        - if policy(@token).show?
+        - if policy(@token).webui_trigger?
           = link_to(token_trigger_path(@token), title: 'Trigger Token') do
             %i.fas.fa-project-diagram
         = link_to '#', title: 'Delete Token',

src/api/app/controllers/webui/users/tokens_controller.rb

@@ -1,5 +1,5 @@
 class Webui::Users::TokensController < Webui::WebuiController
-  before_action :set_token, only: [:edit, :update, :destroy]
+  before_action :set_token, only: [:edit, :update, :destroy, :show]
   before_action :set_parameters, :set_package, only: [:create]
 
   after_action :verify_authorized, except: :index
@@ -49,13 +49,14 @@ def create
     authorize @token
 
     respond_to do |format|
-      format.js do
+      format.html do
         if @token.save
-          flash.now[:success] = "Token successfully created! Make sure you save it - you won't be able to access it again."
-          render partial: 'create', locals: { string: @token.string }
+          flash[:success] = "Token successfully created! Make sure you save it - you won't be able to access it again."
+          session[:show_token] = 'true'
+          redirect_to token_path(@token)
         else
-          flash.now[:error] = "Failed to create token: #{@token.errors.full_messages.to_sentence}."
-          render partial: 'create'
+          flash[:error] = "Failed to create token: #{@token.errors.full_messages.to_sentence}."
+          render :new
         end
       end
     end
@@ -68,6 +69,10 @@ def destroy
     redirect_to tokens_url
   end
 
+  def show
+    authorize @token
+  end
+
   private
 
   def set_token
@@ -97,20 +102,20 @@ def set_package
 
     # Check if only project_name or only package_name are present
     if @extra_params[:project_name].present? ^ @extra_params[:package_name].present?
-      flash.now[:error] = 'When providing an optional package, both Project name and Package name must be provided.'
-      render partial: 'create' and return
+      flash[:error] = 'When providing an optional package, both Project name and Package name must be provided.'
+      render :new and return
     end
 
     # If both project_name and package_name are present, check if this is an existing package
     begin
       @package = Package.get_by_project_and_name(@extra_params[:project_name], @extra_params[:package_name])
     rescue Project::UnknownObjectError
-      flash.now[:error] = "When providing an optional package, the package must exist. Project '#{elide(@extra_params[:project_name])}' does not exist."
-      render partial: 'create'
+      flash[:error] = "When providing an optional package, the package must exist. Project '#{elide(@extra_params[:project_name])}' does not exist."
+      render :new
     rescue Package::UnknownObjectError
-      flash.now[:error] = 'When providing an optional package, the package must exist. ' \
-                          "Package '#{elide(@extra_params[:project_name])}/#{elide(@extra_params[:package_name])}' does not exist."
-      render partial: 'create'
+      flash[:error] = 'When providing an optional package, the package must exist. ' \
+                      "Package '#{elide(@extra_params[:project_name])}/#{elide(@extra_params[:package_name])}' does not exist."
+      render :new
     end
   end
 end

src/api/app/policies/token_policy.rb

@@ -43,6 +43,6 @@ def webui_trigger?
   end
 
   def show?
-    webui_trigger?
+    record.user == user && !record.type.in?(['Token::Rss'])
   end
 end

src/api/app/views/webui/users/tokens/_actions.html.haml

@@ -1 +1 @@
-= link_to('Back to List of Tokens', tokens_path, title: 'Back to List of Tokens', class: 'btn btn-outline-secondary px-4 mt-3 mt-sm-0')
+= link_to('Back to the list of tokens', tokens_path, title: 'Back to the list of tokens', class: 'btn btn-outline-secondary btn-sm')

src/api/app/views/webui/users/tokens/_breadcrumb_items.html.haml

@@ -5,7 +5,7 @@
 - when 'index'
   %li.breadcrumb-item.active{ 'aria-current' => 'page' }
     Tokens
-- when 'new'
+- when 'new', 'create'
   %li.breadcrumb-item
     = link_to 'Tokens', tokens_path
   %li.breadcrumb-item.active{ 'aria-current' => 'page' }
@@ -15,3 +15,8 @@
     = link_to 'Tokens', tokens_path
   %li.breadcrumb-item.active{ 'aria-current' => 'page' }
     Edit Token
+- when 'show'
+  %li.breadcrumb-item
+    = link_to 'Tokens', tokens_path
+  %li.breadcrumb-item.active{ 'aria-current' => 'page' }
+    Show Token

src/api/app/views/webui/users/tokens/edit.html.haml

@@ -12,7 +12,7 @@
               %fieldset.form-group
                 = f.label(:string_readonly, 'Your new OBS Personal Access Token Secret', class: 'col-form-label col-form-label-lg')
                 .input-group
-                  = render CopyToClipboardInputComponent.new(form_builder: f)
+                  = render CopyToClipboardInputComponent.new(token_string: @token.string)
               %hr
 
           .form-row

src/api/app/views/webui/users/tokens/new.html.haml

@@ -7,24 +7,7 @@
 
     .row
       .col-12.col-md-10.col-lg-8
-        = form_with(model: @token, method: :post, local: false) do |f|
-          .visible-when-created.d-none
-            %p.font-weight-bold Your new OBS Personal Access Token
-
-            .form-row#token-id
-              .col-sm
-                .form-group.mb-0
-                  = f.label(:id, 'Id:')
-                  %span= @token.id
-
-            .form-row#created-token
-              .col-12.col-md-10.col-lg-9
-                %fieldset.form-group
-                  = f.label(:string_readonly, 'Secret:', class: 'col-form-label')
-                  .input-group
-                    = render CopyToClipboardInputComponent.new(form_builder: f)
-                %hr
-
+        = form_with(model: @token ||= Token.new, method: :post, local: true) do |f|
           .form-row
             .col
               .form-group

src/api/app/views/webui/users/tokens/show.html.haml

@@ -0,0 +1,68 @@
+- @pagetitle = "Token - #{@token.name.presence || 'No name'}"
+
+.card
+  .card-body
+    %h3.mb-3
+      = @pagetitle
+
+    .row
+      .col-12.col-md-10.col-lg-8
+        - if session[:show_token]
+          %p.font-weight-bold Your new OBS Personal Access Token
+
+        .form-row
+          .col-sm
+            %label.font-weight-bold
+              Id:
+            %span
+              = @token.id
+
+        - if session[:show_token]
+          - session[:show_token] = nil # Ensure we're only showing the token once
+          .form-row
+            .col-12.col-md-10.col-lg-9
+              %fieldset.form-group
+                %label.col-form-label.font-weight-bold
+                  Secret:
+                .input-group
+                  = render CopyToClipboardInputComponent.new(token_string: @token.string)
+              %hr
+
+        .form-row
+          .col
+            .form-group
+              %label.col-form-label.mr-2.font-weight-bold
+                Type:
+              %span
+                = @token.token_name
+
+        - if @token.package
+          .form-row
+            .col-sm
+              .form-group.ui-front
+                %label.font-weight-bold
+                  Project:
+                %span
+                  = link_to(@token.package.project.name, project_show_path(@token.package.project))
+
+          .form-row
+            .col-sm
+              .form-group.ui-front
+                %label.font-weight-bold
+                  Package:
+                %span
+                  = link_to(@token.package.name, package_show_path(@token.package.project, @token.package))
+
+        .actions
+          - if policy(@token).edit?
+            = link_to(edit_token_path(@token), class: 'pl-2', title: 'Edit') do
+              %i.fas.fa-edit
+              %span.nav-item-name Edit
+          - if policy(@token).webui_trigger?
+            = link_to(token_trigger_path(@token), class: 'pl-2', title: 'Trigger Token') do
+              %i.fas.fa-project-diagram
+              %span.nav-item-name Trigger Token
+          - if @token.type == 'Token::Workflow'
+            = link_to(token_workflow_runs_path(@token.id), class: 'pl-2', title: 'Workflow Runs') do
+              %i.fas.fa-book-open
+              %span.nav-item-name Workflow Runs

src/api/spec/controllers/webui/users/tokens_controller_spec.rb

@@ -58,6 +58,7 @@
       let(:form_parameters) { { token: { type: 'runservice', name: 'My first token' } } }
 
       include_examples 'check for flashing a success'
+      it { is_expected.to redirect_to(token_path(Token.last)) }
     end
 
     context 'type is release, with project parameter, without package parameter' do
@@ -74,6 +75,7 @@
       include_examples 'check for flashing a success'
 
       it { expect { subject }.to change(Token, :count).from(0).to(1) }
+      it { is_expected.to redirect_to(token_path(Token.last)) }
     end
 
     context 'type is workflow' do
@@ -83,6 +85,7 @@
         include_examples 'check for flashing a success'
 
         it { expect { subject }.to change(Token, :count).from(0).to(1) }
+        it { is_expected.to redirect_to(token_path(Token.last)) }
       end
 
       context 'without SCM' do

src/api/spec/policies/token_policy_spec.rb

@@ -13,8 +13,15 @@
   permissions :webui_trigger?, :show? do
     it { is_expected.not_to permit(other_user, user_token) }
     it { is_expected.not_to permit(token_user, rss_token) }
-    it { is_expected.not_to permit(token_user, workflow_token) }
 
     it { is_expected.to permit(token_user, user_token) }
   end
+
+  permissions :show? do
+    it { is_expected.to permit(token_user, workflow_token) }
+  end
+
+  permissions :webui_trigger? do
+    it { is_expected.not_to permit(token_user, workflow_token) }
+  end
 end