From 5dfdec645313e556413147597138a8008bc35996 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Mon, 23 Sep 2024 09:37:32 -0400 Subject: [PATCH] Convert `settings/applications` controller spec to system/request specs (#32006) --- .../settings/applications/_fields.html.haml | 36 ---- .../settings/applications/_form.html.haml | 36 ++++ app/views/settings/applications/new.html.haml | 6 +- .../settings/applications/show.html.haml | 6 +- .../settings/applications_controller_spec.rb | 178 ------------------ spec/requests/settings/applications_spec.rb | 44 +++++ spec/system/settings/applications_spec.rb | 144 ++++++++++++++ 7 files changed, 230 insertions(+), 220 deletions(-) delete mode 100644 app/views/settings/applications/_fields.html.haml create mode 100644 app/views/settings/applications/_form.html.haml delete mode 100644 spec/controllers/settings/applications_controller_spec.rb create mode 100644 spec/requests/settings/applications_spec.rb create mode 100644 spec/system/settings/applications_spec.rb diff --git a/app/views/settings/applications/_fields.html.haml b/app/views/settings/applications/_fields.html.haml deleted file mode 100644 index d539848952..0000000000 --- a/app/views/settings/applications/_fields.html.haml +++ /dev/null @@ -1,36 +0,0 @@ -.fields-group - = f.input :name, - label: t('activerecord.attributes.doorkeeper/application.name'), - wrapper: :with_label - -.fields-group - = f.input :website, - label: t('activerecord.attributes.doorkeeper/application.website'), - wrapper: :with_label - -.fields-group - = f.input :redirect_uri, - label: t('activerecord.attributes.doorkeeper/application.redirect_uri'), hint: t('doorkeeper.applications.help.redirect_uri'), - required: true, - wrapper: :with_block_label - - %p.hint= t('doorkeeper.applications.help.native_redirect_uri', native_redirect_uri: content_tag(:code, Doorkeeper.configuration.native_redirect_uri)).html_safe - -.field-group - .input.with_block_label - %label= t('activerecord.attributes.doorkeeper/application.scopes') - %span.hint= t('simple_form.hints.defaults.scopes') - - - Doorkeeper.configuration.scopes.group_by { |s| s.split(':').first }.each_value do |value| - = f.input :scopes, - as: :check_boxes, - collection_wrapper_tag: 'ul', - collection: value.sort, - hint: false, - include_blank: false, - item_wrapper_tag: 'li', - label_method: ->(scope) { safe_join([content_tag(:samp, scope, class: class_for_scope(scope)), content_tag(:span, t("doorkeeper.scopes.#{scope}"), class: 'hint')]) }, - label: false, - required: false, - selected: f.object.scopes.all, - wrapper: :with_block_label diff --git a/app/views/settings/applications/_form.html.haml b/app/views/settings/applications/_form.html.haml new file mode 100644 index 0000000000..66ea8bc12b --- /dev/null +++ b/app/views/settings/applications/_form.html.haml @@ -0,0 +1,36 @@ +.fields-group + = form.input :name, + label: t('activerecord.attributes.doorkeeper/application.name'), + wrapper: :with_label + +.fields-group + = form.input :website, + label: t('activerecord.attributes.doorkeeper/application.website'), + wrapper: :with_label + +.fields-group + = form.input :redirect_uri, + label: t('activerecord.attributes.doorkeeper/application.redirect_uri'), hint: t('doorkeeper.applications.help.redirect_uri'), + required: true, + wrapper: :with_block_label + + %p.hint= t('doorkeeper.applications.help.native_redirect_uri', native_redirect_uri: content_tag(:code, Doorkeeper.configuration.native_redirect_uri)).html_safe + +.field-group + .input.with_block_label + %label= t('activerecord.attributes.doorkeeper/application.scopes') + %span.hint= t('simple_form.hints.defaults.scopes') + + - Doorkeeper.configuration.scopes.group_by { |s| s.split(':').first }.each_value do |value| + = form.input :scopes, + as: :check_boxes, + collection_wrapper_tag: 'ul', + collection: value.sort, + hint: false, + include_blank: false, + item_wrapper_tag: 'li', + label_method: ->(scope) { safe_join([content_tag(:samp, scope, class: class_for_scope(scope)), content_tag(:span, t("doorkeeper.scopes.#{scope}"), class: 'hint')]) }, + label: false, + required: false, + selected: form.object.scopes.all, + wrapper: :with_block_label diff --git a/app/views/settings/applications/new.html.haml b/app/views/settings/applications/new.html.haml index aa2281fea1..f6a546e8de 100644 --- a/app/views/settings/applications/new.html.haml +++ b/app/views/settings/applications/new.html.haml @@ -1,8 +1,8 @@ - content_for :page_title do = t('doorkeeper.applications.new.title') -= simple_form_for @application, url: settings_applications_path do |f| - = render 'fields', f: f += simple_form_for @application, url: settings_applications_path do |form| + = render form .actions - = f.button :button, t('doorkeeper.applications.buttons.submit'), type: :submit + = form.button :button, t('doorkeeper.applications.buttons.submit'), type: :submit diff --git a/app/views/settings/applications/show.html.haml b/app/views/settings/applications/show.html.haml index be1d13eae6..19630cf49b 100644 --- a/app/views/settings/applications/show.html.haml +++ b/app/views/settings/applications/show.html.haml @@ -23,8 +23,8 @@ %hr/ -= simple_form_for @application, url: settings_application_path(@application), method: :put do |f| - = render 'fields', f: f += simple_form_for @application, url: settings_application_path(@application), method: :put do |form| + = render form .actions - = f.button :button, t('generic.save_changes'), type: :submit + = form.button :button, t('generic.save_changes'), type: :submit diff --git a/spec/controllers/settings/applications_controller_spec.rb b/spec/controllers/settings/applications_controller_spec.rb deleted file mode 100644 index 721741bacb..0000000000 --- a/spec/controllers/settings/applications_controller_spec.rb +++ /dev/null @@ -1,178 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Settings::ApplicationsController do - render_views - - let!(:user) { Fabricate(:user) } - let!(:app) { Fabricate(:application, owner: user) } - - before do - sign_in user, scope: :user - end - - describe 'GET #index' do - before do - Fabricate(:application) - get :index - end - - it 'returns http success with private cache control headers', :aggregate_failures do - expect(response).to have_http_status(200) - expect(response.headers['Cache-Control']).to include('private, no-store') - end - end - - describe 'GET #show' do - it 'returns http success' do - get :show, params: { id: app.id } - expect(response).to have_http_status(200) - expect(assigns[:application]).to eql(app) - end - - it 'returns 404 if you dont own app' do - app.update!(owner: nil) - - get :show, params: { id: app.id } - expect(response).to have_http_status 404 - end - end - - describe 'GET #new' do - it 'returns http success' do - get :new - expect(response).to have_http_status(200) - end - end - - describe 'POST #create' do - context 'when success (passed scopes as a String)' do - subject do - post :create, params: { - doorkeeper_application: { - name: 'My New App', - redirect_uri: 'urn:ietf:wg:oauth:2.0:oob', - website: 'http://google.com', - scopes: 'read write follow', - }, - } - end - - it 'creates an entry in the database', :aggregate_failures do - expect { subject }.to change(Doorkeeper::Application, :count) - expect(response).to redirect_to(settings_applications_path) - end - end - - context 'when success (passed scopes as an Array)' do - subject do - post :create, params: { - doorkeeper_application: { - name: 'My New App', - redirect_uri: 'urn:ietf:wg:oauth:2.0:oob', - website: 'http://google.com', - scopes: %w(read write follow), - }, - } - end - - it 'creates an entry in the database', :aggregate_failures do - expect { subject }.to change(Doorkeeper::Application, :count) - expect(response).to redirect_to(settings_applications_path) - end - end - - context 'with failure request' do - before do - post :create, params: { - doorkeeper_application: { - name: '', - redirect_uri: '', - website: '', - scopes: [], - }, - } - end - - it 'returns http success and renders form', :aggregate_failures do - expect(response).to have_http_status(200) - expect(response).to render_template(:new) - end - end - end - - describe 'PATCH #update' do - context 'when success' do - subject do - patch :update, params: { - id: app.id, - doorkeeper_application: opts, - } - response - end - - let(:opts) do - { - website: 'https://foo.bar/', - } - end - - it 'updates existing application' do - subject - - expect(app.reload.website).to eql(opts[:website]) - expect(response).to redirect_to(settings_application_path(app)) - end - end - - context 'with failure request' do - before do - patch :update, params: { - id: app.id, - doorkeeper_application: { - name: '', - redirect_uri: '', - website: '', - scopes: [], - }, - } - end - - it 'returns http success and renders form', :aggregate_failures do - expect(response).to have_http_status(200) - expect(response).to render_template(:show) - end - end - end - - describe 'destroy' do - let(:redis_pipeline_stub) { instance_double(Redis::Namespace, publish: nil) } - let!(:access_token) { Fabricate(:accessible_access_token, application: app) } - - before do - allow(redis).to receive(:pipelined).and_yield(redis_pipeline_stub) - post :destroy, params: { id: app.id } - end - - it 'redirects back to applications page removes the app' do - expect(response).to redirect_to(settings_applications_path) - expect(Doorkeeper::Application.find_by(id: app.id)).to be_nil - end - - it 'sends a session kill payload to the streaming server' do - expect(redis_pipeline_stub).to have_received(:publish).with("timeline:access_token:#{access_token.id}", '{"event":"kill"}') - end - end - - describe 'regenerate' do - let(:token) { user.token_for_app(app) } - - it 'creates new token' do - expect(token).to_not be_nil - post :regenerate, params: { id: app.id } - - expect(user.token_for_app(app)).to_not eql(token) - end - end -end diff --git a/spec/requests/settings/applications_spec.rb b/spec/requests/settings/applications_spec.rb new file mode 100644 index 0000000000..8a5b3de895 --- /dev/null +++ b/spec/requests/settings/applications_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Settings / Exports' do + let(:user) { Fabricate :user } + + before { sign_in user } + + describe 'GET /settings/application/:id' do + context 'when user does not own application' do + let(:application) { Fabricate :application } + + it 'returns http missing' do + get settings_application_path(application) + + expect(response) + .to have_http_status(404) + end + end + end + + describe 'POST /settings/applications' do + subject { post '/settings/applications', params: params } + + let(:params) do + { + doorkeeper_application: { + name: 'My New App', + redirect_uri: 'urn:ietf:wg:oauth:2.0:oob', + website: 'http://google.com', + scopes: 'read write follow', + }, + } + end + + it 'supports passing scope values as string' do + expect { subject } + .to change(Doorkeeper::Application, :count).by(1) + expect(response) + .to redirect_to(settings_applications_path) + end + end +end diff --git a/spec/system/settings/applications_spec.rb b/spec/system/settings/applications_spec.rb new file mode 100644 index 0000000000..ee43da3d5d --- /dev/null +++ b/spec/system/settings/applications_spec.rb @@ -0,0 +1,144 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Settings applications page' do + let!(:application) { Fabricate :application, owner: user } + let(:user) { Fabricate :user } + + before { sign_in user } + + describe 'Viewing the list of applications' do + it 'sees the applications' do + visit settings_applications_path + + expect(page) + .to have_content(application.name) + .and have_private_cache_control + end + end + + describe 'Viewing a single application' do + it 'shows a page with application details' do + visit settings_application_path(application) + + expect(page) + .to have_content(application.name) + end + end + + describe 'Creating a new application' do + it 'accepts form input to make an application' do + visit new_settings_application_path + + fill_in_form + + expect { submit_form } + .to change(Doorkeeper::Application, :count).by(1) + expect(page) + .to have_content(I18n.t('doorkeeper.applications.index.title')) + .and have_content('My new app') + end + + it 'does not save with invalid form values' do + visit new_settings_application_path + + expect { submit_form } + .to not_change(Doorkeeper::Application, :count) + expect(page) + .to have_content("can't be blank") + end + + def fill_in_form + fill_in form_app_name_label, + with: 'My new app' + fill_in I18n.t('activerecord.attributes.doorkeeper/application.website'), + with: 'http://google.com' + fill_in I18n.t('activerecord.attributes.doorkeeper/application.redirect_uri'), + with: 'urn:ietf:wg:oauth:2.0:oob' + + check 'read', id: :doorkeeper_application_scopes_read + check 'write', id: :doorkeeper_application_scopes_write + check 'follow', id: :doorkeeper_application_scopes_follow + end + + def submit_form + click_on I18n.t('doorkeeper.applications.buttons.submit') + end + end + + describe 'Updating an application' do + it 'successfully updates with valid values' do + visit settings_application_path(application) + + fill_in form_app_name_label, + with: 'My new app name with a new value' + submit_form + + expect(page) + .to have_content('My new app name with a new value') + end + + it 'does not update with wrong values' do + visit settings_application_path(application) + + fill_in form_app_name_label, + with: '' + submit_form + + expect(page) + .to have_content("can't be blank") + end + + def submit_form + click_on I18n.t('generic.save_changes') + end + end + + describe 'Destroying an application' do + let(:redis_pipeline_stub) { instance_double(Redis::Namespace, publish: nil) } + let!(:access_token) { Fabricate(:accessible_access_token, application: application) } + + before { stub_redis_pipeline } + + it 'destroys the record and tells the broader universe about that' do + visit settings_applications_path + + expect { destroy_application } + .to change(Doorkeeper::Application, :count).by(-1) + expect(page) + .to have_no_content(application.name) + expect(redis_pipeline_stub) + .to have_received(:publish).with("timeline:access_token:#{access_token.id}", '{"event":"kill"}') + end + + def destroy_application + click_on I18n.t('doorkeeper.applications.index.delete') + end + + def stub_redis_pipeline + allow(redis) + .to receive(:pipelined) + .and_yield(redis_pipeline_stub) + end + end + + describe 'Regenerating an app token' do + it 'updates the app token' do + visit settings_application_path(application) + + expect { regenerate_token } + .to(change { user.token_for_app(application) }) + expect(page) + .to have_content(I18n.t('applications.token_regenerated')) + end + + def regenerate_token + click_on I18n.t('applications.regenerate_token') + end + end + + def form_app_name_label + I18n.t('activerecord.attributes.doorkeeper/application.name') + end +end