[#50212] Convert file storage edit view to primer design #13866

akabiru · 2023-10-06T15:27:20Z

https://community.openproject.org/wp/50212

storage view
- Action buttons (edit, resync OAuth)
- Check with product on Cancel button on storage view (work_packages/50499)
- 🚧 Fix Delete button (https://community.openproject.org/wp/50739)
- 🚧 On Storage name, update PageHeader title (https://community.openproject.org/wp/50739)
Edit storage host form
Resync OpenProject OAuth credentials
- ? TODO: Make Done, continue button context aware
Resync Nextcloud OAuth credentials
Managed project folders form

Primer Components

Extract OpTurbo::FrameComponent [Op#50212]: Add Turbo Stream and Frame View Components #13932
Create task for form input widths handling (https://community.openproject.org/wp/50740)
Create maintenance task to extract OpPrimer::ClipboardCopyComponent and primerise sizes (https://community.openproject.org/wp/50741)
Restrict storage switching in Edit page
Adopt Apply errors from contract validations to model #13952 once ready (https://community.openproject.org/wp/50742)

This reverts commit 5d2cb41.

…model errors to the service

spec/components/op_turbo/frame_wrapper_component_spec.rb

app/components/op_turbo/frame_wrapper_component.rb

…text` https://github.com/opf/openproject/pull/13866/files#r1356978232

… copy

…ove wrapper GitHub Discussion: #13866 (comment) https://community.openproject.org/wp/50212

app/components/open_project/common/clipboard_copy_component.rb

app/services/service_result_errors_presenter.rb

modules/storages/app/controllers/storages/admin/storages_controller.rb

Kharonus

In general I'm impressed by the work. good progress.

There is only a couple of issues I'd like to discuss or mention. But none of those are blockers for a merge from my pov.

modules/storages/app/components/storages/admin/storage_list_component.html.erb

modules/storages/config/locales/en.yml

modules/storages/spec/contracts/storages/storages/nextcloud_contract_spec.rb

modules/storages/spec/features/admin_storages_spec.rb

This reverts commit 03dd1a0.

…n directly 03dd1a0#r1371602305

…-primer-design

…el errors https://community.openproject.org/work_packages/50742

HDinger

Hi @akabiru

nice job, it looks really awesome! I did not have a complete storage setup, so I could test only half of the things. It might be that some of the issues I found are a result of this half setup.

When I try to add a new storage, I get a black screen with a weird message of a missing parameter as soon as I press "Save":

When trying to refresh an incomplete OAuthApplication via the arrows on the right, I get a routing error.

On mobile, there are some overlappings

Compared to the visuals, there is no breadcrumb shown for me. I asuume that is part of the other PR, right?

As I said, I don't know if these are actual issues or come due to my hacked setup. If you have problems reproducing the issues, I can also show you tomorrow.

modules/storages/app/components/storages/admin/storage_view_component.html.erb

HDinger

Hi @akabiru

I was able to test it better now. Some additional remarks I found:

Deleting the storage results in a routing error,
When saving the automatically saved folders, the section does not collapse, but stays open

akabiru · 2023-10-26T11:23:04Z

Hi @akabiru

I was able to test it better now. Some additional remarks I found:
* [ ]  Deleting the storage results in a routing error,

Thanks @HDinger, the delete problem is a known issue unfortunately captured separately in (https://community.openproject.org/projects/openproject/work_packages/50739) - but I understand the expecation for it to work in this PR- I'll look into it 🔧

* [ ]  When saving the automatically saved folders, the section does not collapse, but stays open

Will reach out directly as I haven't observed this problem

…uest https://community.openproject.org/work_packages/50739

…tation

akabiru

Thanks for the review @HDinger 🙌🏾

Resolved

When I try to add a new storage, I get a black screen with a weird message of a missing parameter as soon as I press "Save":
c9d8297
When trying to refresh an incomplete OAuthApplication via the arrows on the right, I get a routing error.
manually created storage that did not have an oauth application
Why do you need a grid if there is only one element inside anyways? 11188f2

🚧 Separate Work Packages: followup work

On mobile, there are some overlappings
https://community.openproject.org/wp/50762
Compared to the visuals, there is no breadcrumb shown for me. I asuume that is part of the other PR, right? ✅ feat[#50499]: Add navigational elements (breadcrumb and back buttons) #13962

aaron-contreras · 2023-10-26T15:27:18Z

modules/storages/app/views/storages/admin/storages/edit.html.erb

+            Primer::Beta::Button.new(
+              scheme: :danger,
+              size: :medium,
+              type: :submit,
+              aria: { label: I18n.t("storages.label_delete_storage") },
+              data: { confirm: I18n.t('storages.delete_warning.storage') },
+              test_selector: 'storage-delete-button'
+            )
+          ) do |button|
+            button.with_leading_visual_icon(icon: :trash)
+            I18n.t('button_delete')
+          end


HDinger

Hi @akabiru

The changes look good to me. 👍 I found one other issue, but I am fine with moving that to a separate ticket if you want to:

When I change the name of a storage, the name is not updated on the page header (only with a hard reload). However, as there is no success message or anything to indicate that the change was accepted, it felt like a bug.

Additionally, I received another routing error when trying to change the name of one of my storages. However, it did happen only for this specifc storage, so that might be due to my setup.

Bildschirmaufnahme.2023-10-27.um.08.06.01.mov

akabiru · 2023-10-27T06:52:43Z

Hi @akabiru

The changes look good to me. 👍 I found one other issue, but I am fine with moving that to a separate ticket if you want to:
* When I change the name of a storage, the name is not updated on the page header (only with a hard reload). However, as there is no success message or anything to indicate that the change was accepted, it felt like a bug.

Thanks @HDinger , the header name update is WIP separately in https://community.openproject.org/wp/50738

Additionally, I received another routing error when trying to change the name of one of my storages. However, it did happen only for this specifc storage, so that might be due to my setup.

Hmm, I suspect this is due to a manually created storage- as https://example.com is not a valid nextcloud URL, and would fail if created via the Form/API I will nonetheless track it as a bug to handle more gracefully. Confirmed if the storage is created manually via console, the dependent oauth application relation is not created which causes the crash above

HDinger

👍 🌟

akabiru added 7 commits October 6, 2023 15:50

chore[#50212]: Begin layout

bfb1232

chore[#50212]: Combine StorageButtonComponent with options

03dd1a0

chore[#50212]: Rename to StorageComponent

5d2cb41

Revert "chore[#50212]: Rename to StorageComponent"

d376a07

This reverts commit 5d2cb41.

feat[#50212]: Retrieve status labels from configuration checks

ab953b5

fix[#50212]: Restore configuration checks component

e8a1d53

fix[#50212]: Remove unused methods

21dda69

akabiru self-assigned this Oct 9, 2023

akabiru added 9 commits October 10, 2023 11:45

chore[#50212]: edit_host_name_configuration blueprint

010c569

feat[#50212]: Define edit provider host form with validation

c9488fd

chore[#50212]: rename to edit_host

f240bf2

chore[#50212]: Define a StorageResultErrorPresenter that delegates …

1c779c5

…model errors to the service

chore[#50212]: Namespace Storage::Admin::Forms

586f46a

chore[#50212]: Remove redundant storage_ prefix

b854a66

chore[#50212]: Remove redundant unused helpers

4200c43

feat[#50212]: Add OpTurbo::FrameWrapperComponent

63df7c5

chore[#50212]: Utilise FrameWrapperComponent in edit host view

dff7dc3

aaron-contreras reviewed Oct 12, 2023

View reviewed changes

spec/components/op_turbo/frame_wrapper_component_spec.rb Outdated Show resolved Hide resolved

cbliard reviewed Oct 12, 2023

View reviewed changes

app/components/op_turbo/frame_wrapper_component.rb Outdated Show resolved Hide resolved

akabiru added 7 commits October 16, 2023 10:29

chore[#50212]: WIP - OAuth Credentials clipboard copy

15ee221

feat[#50212]: Add OpenProject::Common::ClipboardCopyComponent

4629956

chore[#50212]: Use clipboard copy component

62eb07f

chore[#50212]: Use primer text selector, set up basic spec

96cfa2b

chore[#50212]: Add flash warning for credentials copy

b138b21

chore[#50212]: Use attr options reader

63692ab

fix[#50212]: Rename OpTurbo::FrameWrapperComponent :frame to `con…

72f557b

…text` https://github.com/opf/openproject/pull/13866/files#r1356978232

akabiru force-pushed the implementation/50212-convert-the-edit-page-to-primer-design branch from 8c040ef to 72f557b Compare October 16, 2023 13:56

fix[#50212]: Adopt primer default to visually hide label in clipboard…

41338aa

… copy

akabiru added a commit that referenced this pull request Oct 17, 2023

chore[#50212]: Define OpTurbo::{FrameComponent,StreamComponent} rem…

c8916d1

…ove wrapper GitHub Discussion: #13866 (comment) https://community.openproject.org/wp/50212

akabiru mentioned this pull request Oct 17, 2023

[Op#50212]: Add Turbo Stream and Frame View Components #13932

Merged

3 tasks

akabiru added the needs review label Oct 24, 2023

akabiru commented Oct 24, 2023

View reviewed changes

app/components/open_project/common/clipboard_copy_component.rb Show resolved Hide resolved

app/services/service_result_errors_presenter.rb Outdated Show resolved Hide resolved

modules/storages/app/controllers/storages/admin/storages_controller.rb Outdated Show resolved Hide resolved

tests[Op#50212]: Expand Storage "Automatically managed view" spec

48a7eda

akabiru force-pushed the implementation/50212-convert-the-edit-page-to-primer-design branch from 1bbda45 to 48a7eda Compare October 25, 2023 09:07

akabiru requested a review from HDinger October 25, 2023 11:30

Kharonus approved these changes Oct 25, 2023

View reviewed changes

akabiru added 2 commits October 25, 2023 16:46

Revert "chore[#50212]: Combine StorageButtonComponent with options"

0a84380

This reverts commit 03dd1a0.

chore[Op#50212]: remove DeleteStorageButtonComponent, use Beta::Butto…

6628edf

…n directly 03dd1a0#r1371602305

akabiru force-pushed the implementation/50212-convert-the-edit-page-to-primer-design branch from 6628edf to b1fec70 Compare October 25, 2023 14:11

Merge branch 'dev' into implementation/50212-convert-the-edit-page-to…

f544fbe

…-primer-design

akabiru force-pushed the implementation/50212-convert-the-edit-page-to-primer-design branch from b1fec70 to f544fbe Compare October 25, 2023 14:17

akabiru added 3 commits October 25, 2023 17:31

Merge branch 'dev' into implementation/50212-convert-the-edit-page-to…

162b111

…-primer-design

fix[Op#50742] Replace StorageErrorPresenter with new contract x mod…

457ed3d

…el errors https://community.openproject.org/work_packages/50742

fix[Op#50212]: Reinstate new storage button component template

14d08ea

akabiru removed the needs review label Oct 25, 2023

HDinger reviewed Oct 25, 2023

View reviewed changes

modules/storages/app/components/storages/admin/storage_view_component.html.erb Outdated Show resolved Hide resolved

HDinger requested changes Oct 26, 2023

View reviewed changes

akabiru added 3 commits October 26, 2023 14:40

fix[#50739] DELETE storage button fails and is converted to a GET req…

f86f0e3

…uest https://community.openproject.org/work_packages/50739

chore[Op#50212]: remove unneeded header grid layout

11188f2

fix[Op#50212]: check for params presence, pending FormObject implemen…

c9d8297

…tation

akabiru requested a review from HDinger October 26, 2023 13:38

akabiru commented Oct 26, 2023

View reviewed changes

aaron-contreras reviewed Oct 26, 2023

View reviewed changes

HDinger reviewed Oct 27, 2023

View reviewed changes

akabiru requested a review from HDinger October 27, 2023 08:45

HDinger approved these changes Oct 27, 2023

View reviewed changes

akabiru merged commit 87ace40 into dev Oct 27, 2023

akabiru deleted the implementation/50212-convert-the-edit-page-to-primer-design branch October 27, 2023 08:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#50212] Convert file storage edit view to primer design #13866

[#50212] Convert file storage edit view to primer design #13866

akabiru commented Oct 6, 2023 •

edited

Loading

Kharonus left a comment

HDinger left a comment •

edited by akabiru

Loading

HDinger left a comment •

edited by akabiru

Loading

akabiru commented Oct 26, 2023

akabiru left a comment •

edited

Loading

aaron-contreras Oct 26, 2023

HDinger left a comment

akabiru commented Oct 27, 2023 •

edited

Loading

HDinger left a comment

[#50212] Convert file storage edit view to primer design #13866

[#50212] Convert file storage edit view to primer design #13866

Conversation

akabiru commented Oct 6, 2023 • edited Loading

https://community.openproject.org/wp/50212

Kharonus left a comment

Choose a reason for hiding this comment

HDinger left a comment • edited by akabiru Loading

Choose a reason for hiding this comment

HDinger left a comment • edited by akabiru Loading

Choose a reason for hiding this comment

akabiru commented Oct 26, 2023

akabiru left a comment • edited Loading

Choose a reason for hiding this comment

aaron-contreras Oct 26, 2023

Choose a reason for hiding this comment

HDinger left a comment

Choose a reason for hiding this comment

akabiru commented Oct 27, 2023 • edited Loading

HDinger left a comment

Choose a reason for hiding this comment

akabiru commented Oct 6, 2023 •

edited

Loading

HDinger left a comment •

edited by akabiru

Loading

HDinger left a comment •

edited by akabiru

Loading

akabiru left a comment •

edited

Loading

akabiru commented Oct 27, 2023 •

edited

Loading