Skip to content

Commit

Permalink
Merge pull request #14215 from opf/implementation/50921-present-the-s…
Browse files Browse the repository at this point in the history
…torage-health-information

#50921 Add storage health information in admin page
  • Loading branch information
dominic-braeunlein authored Nov 28, 2023
2 parents 908c922 + 2221117 commit 839c40f
Show file tree
Hide file tree
Showing 9 changed files with 218 additions and 13 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<%= render(Primer::Box.new) do %>
<%= render(Primer::Beta::Heading.new(tag: :h4, mb: 2)) { I18n.t('storages.health.title') } %>
<%= render(Primer::Box.new(py: 1)) do %>
<%= render(Primer::Beta::Text.new(test_selector: "storage-health-changed-at")) { I18n.t('storages.health.checked', datetime: helpers.format_time(storage.health_changed_at)) } %>
<% if storage.health_healthy? %>
<%= render(Primer::Beta::Label.new(scheme: :success, test_selector: "storage-health-label-healthy")) {
I18n.t('storages.health.label_healthy')
} %>
<% elsif storage.health_unhealthy? %>
<%= render(Primer::Beta::Label.new(scheme: :danger, test_selector: "storage-health-label-error")) {
I18n.t('storages.health.label_error')
} %>
<% else %>
<%= render(Primer::Beta::Label.new(scheme: :attention, test_selector: "storage-health-label-pending")) {
I18n.t('storages.health.label_pending')
} %>
<% end %>
<% end %>
<% if storage.health_unhealthy? %>
<%= render(Primer::Beta::Text.new(tag: :div, color: :muted, py: 1, test_selector: "storage-health-reason")) { storage.formatted_health_reason } %>
<% end %>
<% end %>
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# frozen_string_literal: true

#-- copyright
# OpenProject is an open source project management software.
# Copyright (C) 2012-2023 the OpenProject GmbH
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
# Copyright (C) 2006-2013 Jean-Philippe Lang
# Copyright (C) 2010-2013 the ChiliProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
#
# See COPYRIGHT and LICENSE files for more details.
#++
#
module Storages::Admin
class HealthStatusComponent < ApplicationComponent
alias_method :storage, :model
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,11 @@
concat(render(Primer::Beta::Link.new(href: edit_admin_settings_storage_path(storage), scheme: :primary, mr: 1, data: { 'test-selector': 'storage-name' })) { storage.name })

unless storage.configured?
concat(render(Primer::Beta::Label.new(scheme: :attention, data: { 'test-selector': 'label-incomplete' })) { I18n.t('storages.label_incomplete') })
concat(render(Primer::Beta::Label.new(scheme: :attention, test_selector: "label-incomplete")) { I18n.t('storages.label_incomplete') })
end

if storage.health_unhealthy?
concat(render(Primer::Beta::Label.new(scheme: :danger, test_selector: "storage-health-label-error")) { I18n.t('storages.health.label_error') })
end
end

Expand Down
12 changes: 12 additions & 0 deletions modules/storages/app/models/storages/storage.rb
Original file line number Diff line number Diff line change
Expand Up @@ -131,5 +131,17 @@ def provider_type_nextcloud?
def provider_type_one_drive?
provider_type == ::Storages::Storage::PROVIDER_TYPE_ONE_DRIVE
end

# Currently the error messages saved look like:
# "unauthorized | Outbound request not authorized | #<Storages::StorageErrorData:0x0000ffff646ac570>"
# This method returns the first two parts of the error message.
def formatted_health_reason
split_reason = health_reason.split('|')
if split_reason.length === 3
"#{split_reason[0].strip.capitalize}: #{split_reason[1]}"
else
health_reason
end
end
end
end
17 changes: 16 additions & 1 deletion modules/storages/app/views/storages/admin/storages/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,19 @@ See COPYRIGHT and LICENSE files for more details.
<% end %>
<% end %>
<%= render(::Storages::Admin::StorageViewComponent.new(@storage)) %>
<% if @storage.provider_type_nextcloud? && @storage.automatically_managed? %>
<%= render(Primer::Alpha::Layout.new(stacking_breakpoint: :lg)) do |component| %>
<% component.with_main() do %>
<%= render(::Storages::Admin::StorageViewComponent.new(@storage)) %>
<% end %>
<% component.with_sidebar(col_placement: :end, row_placement: :end) do %>
<%= render(Primer::OpenProject::BorderGrid.new(spacious: true)) do |grid| %>
<% grid.with_row do %>
<%= render(::Storages::Admin::HealthStatusComponent.new(@storage)) %>
<% end %>
<% end %>
<% end %>
<% end %>
<% else %>
<%= render(::Storages::Admin::StorageViewComponent.new(@storage)) %>
<% end %>
6 changes: 6 additions & 0 deletions modules/storages/config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -248,3 +248,9 @@ en:
automatically_managed_project_folder_missing: "To complete the setup, please configure automatically managed project folders for your storage."
notice_oauth_application_replaced: "The OpenProject OAuth application was successfully replaced."
notice_successful_storage_connection: "Storage connected successfully! Remember to activate the module and the specific storage in the project settings of each desired project to use it."
health:
title: "Managed folders status"
label_pending: "Pending"
label_error: "Error"
label_healthy: "Healthy"
checked: "Checked %{datetime}"
37 changes: 37 additions & 0 deletions modules/storages/spec/factories/storage_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
class: '::Storages::NextcloudStorage' do
provider_type { Storages::Storage::PROVIDER_TYPE_NEXTCLOUD }
sequence(:host) { |n| "https://host#{n}.example.com" }
sequence(:name) { |n| "Storage #{n}" }

trait :as_automatically_managed do
automatically_managed { true }
Expand All @@ -64,6 +65,30 @@
trait :as_not_automatically_managed do
automatically_managed { false }
end

trait :as_healthy do
health_status { 'healthy' }
health_reason { nil }
health_changed_at { Time.now.utc }
end

trait :as_unhealthy do
health_status { 'unhealthy' }
health_reason { 'error reason' }
health_changed_at { Time.now.utc }
end

trait :as_unhealthy_long_reason do
health_status { 'unhealthy' }
health_reason { 'unauthorized | Outbound request not authorized | #<Storages::StorageErrorData:0x0000ffff646ac570>' }
health_changed_at { Time.now.utc }
end

trait :as_pending do
health_status { 'pending' }
health_reason { nil }
health_changed_at { Time.now.utc }
end
end

factory :nextcloud_storage_with_local_connection,
Expand Down Expand Up @@ -106,6 +131,18 @@
end
end

factory :nextcloud_storage_with_complete_configuration,
parent: :nextcloud_storage,
traits: [:as_automatically_managed] do
sequence(:host) { |n| "https://host-complete#{n}.example.com" }
sequence(:name) { |n| "Storage complete #{n}" }

after(:create) do |storage|
create(:oauth_client, integration: storage)
create(:oauth_application, integration: storage)
end
end

factory :one_drive_storage,
parent: :storage,
class: '::Storages::OneDriveStorage' do
Expand Down
92 changes: 82 additions & 10 deletions modules/storages/spec/features/admin_storages_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
RSpec.describe 'Admin storages',
:js,
:storage_server_helpers do

let(:admin) { create(:admin) }

before { login_as admin }
Expand All @@ -43,34 +44,59 @@
let(:complete_storage) { create(:nextcloud_storage_with_local_connection) }
let(:incomplete_storage) { create(:nextcloud_storage) }

let(:complete_nextcloud_storage_health_pending) { create(:nextcloud_storage_with_complete_configuration) }
let(:complete_nextcloud_storage_health_healthy) { create(:nextcloud_storage_with_complete_configuration, :as_healthy) }
let(:complete_nextcloud_storage_health_unhealthy) { create(:nextcloud_storage_with_complete_configuration, :as_unhealthy) }

before do
complete_storage
incomplete_storage
complete_nextcloud_storage_health_pending
complete_nextcloud_storage_health_healthy
complete_nextcloud_storage_health_unhealthy
end

it 'renders a list of storages' do
visit admin_settings_storages_path

expect(page).to have_css('[data-test-selector="storage-name"]', text: complete_storage.name)
expect(page).to have_css('[data-test-selector="storage-name"]', text: incomplete_storage.name)
expect(page).to have_test_selector('storage-name', text: complete_storage.name)
expect(page).to have_test_selector('storage-name', text: incomplete_storage.name)
expect(page).to have_css("a[role='button'][aria-label='Add new storage'][href='#{new_admin_settings_storage_path}']",
text: 'Storage')

within "li#storages_nextcloud_storage_#{complete_storage.id}" do
expect(page).not_to have_css('[data-test-selector="label-incomplete"]')
expect(page).not_to have_test_selector('label-incomplete')
expect(page).to have_link(complete_storage.name, href: edit_admin_settings_storage_path(complete_storage))
expect(page).to have_css('[data-test-selector="storage-creator"]', text: complete_storage.creator.name)
expect(page).to have_css('[data-test-selector="storage-provider"]', text: 'Nextcloud')
expect(page).to have_css('[data-test-selector="storage-host"]', text: complete_storage.host)
expect(page).to have_test_selector('storage-creator', text: complete_storage.creator.name)
expect(page).to have_test_selector('storage-provider', text: 'Nextcloud')
expect(page).to have_test_selector('storage-host', text: complete_storage.host)
end

within "li#storages_nextcloud_storage_#{incomplete_storage.id}" do
expect(page).to have_css('[data-test-selector="label-incomplete"]')
expect(page).to have_css('[data-test-selector="storage-name"]', text: incomplete_storage.name)
expect(page).to have_css('[data-test-selector="storage-provider"]', text: 'Nextcloud')
expect(page).to have_css('[data-test-selector="storage-host"]', text: incomplete_storage.host)
expect(page).to have_test_selector('label-incomplete')
expect(page).to have_test_selector('storage-name', text: incomplete_storage.name)
expect(page).to have_test_selector('storage-provider', text: 'Nextcloud')
expect(page).to have_test_selector('storage-host', text: incomplete_storage.host)
expect(page).to have_css('.op-principal--name', text: incomplete_storage.creator.name)
end

within "li#storages_nextcloud_storage_#{complete_nextcloud_storage_health_pending.id}" do
expect(page).not_to have_test_selector('storage-health-label-error')
expect(page).not_to have_test_selector('storage-health-label-healthy')
expect(page).not_to have_test_selector('storage-health-label-pending')
end

within "li#storages_nextcloud_storage_#{complete_nextcloud_storage_health_healthy.id}" do
expect(page).not_to have_test_selector('storage-health-label-healthy')
expect(page).not_to have_test_selector('storage-health-label-error')
expect(page).not_to have_test_selector('storage-health-label-pending')
end

within "li#storages_nextcloud_storage_#{complete_nextcloud_storage_health_unhealthy.id}" do
expect(page).to have_test_selector('storage-health-label-error')
expect(page).not_to have_test_selector('storage-health-label-healthy')
expect(page).not_to have_test_selector('storage-health-label-pending')
end
end
end

Expand Down Expand Up @@ -543,4 +569,50 @@
end
end
end

describe 'Health status information in edit page' do
frozen_date_time = Time.zone.local(2023, 11, 28, 1, 2, 3)

let(:complete_nextcloud_storage_health_pending) { create(:nextcloud_storage_with_complete_configuration) }
let(:complete_nextcloud_storage_health_healthy) { create(:nextcloud_storage_with_complete_configuration, :as_healthy) }
let(:complete_nextcloud_storage_health_unhealthy) { create(:nextcloud_storage_with_complete_configuration, :as_unhealthy) }
let(:complete_nextcloud_storage_health_unhealthy_long_reason) do
create(:nextcloud_storage_with_complete_configuration, :as_unhealthy_long_reason)
end

before do
Timecop.freeze(frozen_date_time)
complete_nextcloud_storage_health_pending
complete_nextcloud_storage_health_healthy
complete_nextcloud_storage_health_unhealthy
complete_nextcloud_storage_health_unhealthy_long_reason
end

after do
Timecop.return
end

it 'shows healthy status for storages that are healthy' do
visit edit_admin_settings_storage_path(complete_nextcloud_storage_health_healthy)
expect(page).to have_test_selector('storage-health-label-healthy', text: 'Healthy')
expect(page).to have_test_selector('storage-health-changed-at', text: "Checked 11/28/2023 01:02 AM")
end

it 'shows pending label for a storage that is pending' do
visit edit_admin_settings_storage_path(complete_nextcloud_storage_health_pending)
expect(page).to have_test_selector('storage-health-label-pending', text: 'Pending')
end

it 'shows error status for storages that are unhealthy' do
visit edit_admin_settings_storage_path(complete_nextcloud_storage_health_unhealthy)
expect(page).to have_test_selector('storage-health-label-error', text: 'Error')
expect(page).to have_test_selector('storage-health-reason', text: 'error reason')
end

it 'shows formatted error reason for storages that are unhealthy' do
visit edit_admin_settings_storage_path(complete_nextcloud_storage_health_unhealthy_long_reason)
expect(page).to have_test_selector('storage-health-label-error', text: 'Error')
expect(page).to have_test_selector('storage-health-reason', text: 'Unauthorized: Outbound request not authorized')
end
end
end
2 changes: 1 addition & 1 deletion spec/factories/oauth_application_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
confidential { true }
owner factory: :admin
owner_type { 'User' }
uid { '12345' }
sequence(:uid) { |n| "2345678901-#{n}" }
redirect_uri { 'urn:ietf:wg:oauth:2.0:oob' }
scopes { 'api_v3' }
end
Expand Down

0 comments on commit 839c40f

Please sign in to comment.