[ENG-6538] Require root folder to be selected when configuring addon (#2412)

futa-ikeda · web-flow · commit 32cfc9c195fb · 2024-11-22T16:02:39.000-05:00
-   Ticket: [ENG-6538]
-   Feature flag: n/a

# TODO
- [x] Allow `getFolderItems` and `getItemInfo` to be done with the `thruAccount` arg (Right now it only uses the `thruAddon` arg which is for configured-addons)
  - [x] Have the `&lt;AddonsService::FileManager&gt;` be able to use either the `configuredAddon` or the `authorizedAccount` to get files
- [x] Prevent configured-addon creation when a user confirms the authorized-account they want to use
- [x] Save or create the configured-addon when user's click the "Save" button in the `&lt;AddonService::ConfiguredAddonEdit&gt;` component
- [x] Update tests

## Purpose
- Disallow users from saving if there is no root folder selected


## Summary of Changes
- Disable "Save" button if no root folder is selected
- Update default behavior to prevent a `configure-addon` from being created until after all the configuring has finished
- Update tests
diff --git a/app/guid-node/addons/index/template.hbs b/app/guid-node/addons/index/template.hbs
@@ -148,11 +148,10 @@
                             <Button
                                 data-test-addon-confirm-setup-button
                                 data-analytics-name='Confirm Setup'
-                                disabled={{manager.confirmAccountSetup.isRunning}}
                                 @type='primary'
-                                {{on 'click' (perform manager.confirmAccountSetup manager.selectedAccount)}}
+                                {{on 'click' manager.confirmAccountSetup}}
                             >
-                                {{if manager.confirmAccountSetup.isRunning (t 'addons.confirm.authorizing') (t 'general.confirm')}}
+                                {{t 'general.confirm'}}
                             </Button>
                         </div>
                     {{else if (eq manager.pageMode 'configurationList')}}
@@ -269,7 +268,8 @@
                     {{else if (eq manager.pageMode 'configure')}}
                         <AddonsService::ConfiguredAddonEdit
                             @configuredAddon={{manager.selectedConfiguration}}
-                            @onSave={{perform manager.saveConfiguration}}
+                            @authorizedAccount={{manager.selectedAccount}}
+                            @onSave={{perform manager.saveOrCreateConfiguration}}
                             @onCancel={{manager.cancelSetup}}
                         />
                     {{/if}}
diff --git a/app/models/addon-operation-invocation.ts b/app/models/addon-operation-invocation.ts
@@ -1,9 +1,22 @@
 import Model, { AsyncBelongsTo, attr, belongsTo } from '@ember-data/model';
 
 import UserReferenceModel from 'ember-osf-web/models/user-reference';
-import ConfiguredAddonModel, { ConnectedOperationNames } from 'ember-osf-web/models/configured-addon';
+import ConfiguredAddonModel from 'ember-osf-web/models/configured-addon';
 import AuthorizedAccountModel from 'ember-osf-web/models/authorized-account';
 
+export enum ConnectedOperationNames {
+    HasRevisions = 'has_revisions',
+    ListRootItems = 'list_root_items',
+    ListChildItems = 'list_child_items',
+    GetItemInfo = 'get_item_info',
+}
+
+export interface OperationKwargs {
+    itemId?: string;
+    itemType?: ItemType;
+    pageCursor?: string;
+}
+
 export enum InvocationStatus {
     STARTING = 'STARTING',
     GOING = 'GOING',
@@ -38,7 +51,7 @@ export interface Item {
 export default class AddonOperationInvocationModel extends Model {
     @attr('string') invocationStatus!: InvocationStatus;
     @attr('string') operationName!: ConnectedOperationNames;
-    @attr('object', {snakifyForApi: true}) operationKwargs!: any;
+    @attr('object', {snakifyForApi: true}) operationKwargs!: Partial<OperationKwargs>;
     @attr('object', {snakifyForApi: true}) operationResult!: OperationResult;
     @attr('date') created!: Date;
     @attr('date') modified!: Date;
diff --git a/app/models/authorized-account.ts b/app/models/authorized-account.ts
@@ -1,4 +1,12 @@
 import Model, { attr } from '@ember-data/model';
+import { waitFor } from '@ember/test-waiters';
+import { task } from 'ember-concurrency';
+import { ConnectedOperationNames, OperationKwargs } from 'ember-osf-web/models/addon-operation-invocation';
+
+export enum ConnectedCapabilities {
+    Access = 'ACCESS',
+    Update = 'UPDATE',
+}
 
 export interface AddonCredentialFields {
     username?: string;
@@ -24,4 +32,29 @@ export default class AuthorizedAccountModel extends Model {
     @attr('boolean') initiateOauth!: boolean; // write-only
     @attr('fixstring') readonly authUrl!: string; // Only returned when POSTing to /authorized-xyz-accounts
     @attr('boolean') readonly credentialsAvailable!: boolean;
+
+    @task
+    @waitFor
+    async getFolderItems(this: AuthorizedAccountModel, kwargs?: OperationKwargs) {
+        const operationKwargs = kwargs || {};
+        const operationName = operationKwargs.itemId ? ConnectedOperationNames.ListChildItems :
+            ConnectedOperationNames.ListRootItems;
+        const newInvocation = this.store.createRecord('addon-operation-invocation', {
+            operationName,
+            operationKwargs,
+            thruAccount: this,
+        });
+        return await newInvocation.save();
+    }
+
+    @task
+    @waitFor
+    async getItemInfo(this: AuthorizedAccountModel, itemId: string) {
+        const newInvocation = this.store.createRecord('addon-operation-invocation', {
+            operationName: ConnectedOperationNames.GetItemInfo,
+            operationKwargs: { itemId },
+            thruAccount: this,
+        });
+        return await newInvocation.save();
+    }
 }
diff --git a/app/models/configured-addon.ts b/app/models/configured-addon.ts
@@ -2,27 +2,11 @@ import Model, { AsyncBelongsTo, attr, belongsTo } from '@ember-data/model';
 import { waitFor } from '@ember/test-waiters';
 import { task } from 'ember-concurrency';
 
-import { ItemType } from './addon-operation-invocation';
+import { ConnectedOperationNames, OperationKwargs } from './addon-operation-invocation';
 import ResourceReferenceModel from './resource-reference';
 import UserReferenceModel from './user-reference';
+import { ConnectedCapabilities } from './authorized-account';
 
-export enum ConnectedCapabilities {
-    Access = 'ACCESS',
-    Update = 'UPDATE',
-}
-
-export enum ConnectedOperationNames {
-    HasRevisions = 'has_revisions',
-    ListRootItems = 'list_root_items',
-    ListChildItems = 'list_child_items',
-    GetItemInfo = 'get_item_info',
-}
-
-export interface OperationKwargs {
-    itemId?: string;
-    itemType?: ItemType;
-    pageCursor?: string;
-}
 export interface ConfiguredAddonEditableAttrs {
     displayName: string;
     rootFolder: string;
diff --git a/app/packages/files/service-file.ts b/app/packages/files/service-file.ts
@@ -6,7 +6,7 @@ import { task } from 'ember-concurrency';
 import Intl from 'ember-intl/services/intl';
 import Toast from 'ember-toastr/services/toast';
 import ConfiguredStorageAddonModel from 'ember-osf-web/models/configured-storage-addon';
-import { ConnectedOperationNames, ConnectedCapabilities } from 'ember-osf-web/models/configured-addon';
+import { ConnectedOperationNames, ConnectedCapabilities } from 'ember-osf-web/models/addon-operation-invocation';
 import FileModel from 'ember-osf-web/models/file';
 import NodeModel from 'ember-osf-web/models/node';
 import { Permission } from 'ember-osf-web/models/osf-model';
diff --git a/app/packages/files/service-provider-file.ts b/app/packages/files/service-provider-file.ts
@@ -12,7 +12,7 @@ import CurrentUserService from 'ember-osf-web/services/current-user';
 import captureException, { getApiErrorMessage } from 'ember-osf-web/utils/capture-exception';
 import { ErrorDocument } from 'osf-api';
 import ConfiguredStorageAddonModel from 'ember-osf-web/models/configured-storage-addon';
-import { ConnectedOperationNames, ConnectedCapabilities } from 'ember-osf-web/models/configured-addon';
+import { ConnectedOperationNames, ConnectedCapabilities } from 'ember-osf-web/models/addon-operation-invocation';
 import ServiceFile from 'ember-osf-web/packages/files/service-file';
 import { ExternalServiceCapabilities } from 'ember-osf-web/models/external-service';
 
diff --git a/lib/osf-components/addon/components/addons-service/configured-addon-edit/component.ts b/lib/osf-components/addon/components/addons-service/configured-addon-edit/component.ts
@@ -4,17 +4,19 @@ import { tracked } from '@glimmer/tracking';
 import { TaskInstance } from 'ember-concurrency';
 
 import { Item, ItemType } from 'ember-osf-web/models/addon-operation-invocation';
+import AuthorizedAccountModel from 'ember-osf-web/models/authorized-account';
 import ConfiguredAddonModel from 'ember-osf-web/models/configured-addon';
 
 
 interface Args {
-    configuredAddon: ConfiguredAddonModel;
+    configuredAddon?: ConfiguredAddonModel;
+    selectedAccount?: AuthorizedAccountModel;
     onSave: TaskInstance<void>;
 }
 
 export default class ConfiguredAddonEdit extends Component<Args> {
-    @tracked displayName = this.args.configuredAddon.displayName;
-    @tracked selectedFolder = this.args.configuredAddon.rootFolder;
+    @tracked displayName = this.args.configuredAddon?.displayName;
+    @tracked selectedFolder = this.args.configuredAddon?.rootFolder;
     @tracked currentItems: Item[] = [];
 
     defaultKwargs = {
@@ -26,7 +28,7 @@ export default class ConfiguredAddonEdit extends Component<Args> {
     }
 
     get disableSave() {
-        return this.invalidDisplayName || this.args.onSave.isRunning;
+        return !this.selectedFolder || this.invalidDisplayName || this.args.onSave.isRunning;
     }
 
     get onSaveArgs() {
diff --git a/lib/osf-components/addon/components/addons-service/configured-addon-edit/template.hbs b/lib/osf-components/addon/components/addons-service/configured-addon-edit/template.hbs
@@ -22,6 +22,7 @@
     </div>
     <AddonsService::FileManager
         @configuredAddon={{@configuredAddon}}
+        @authorizedAccount={{@authorizedAccount}}
         @defaultKwargs={{this.defaultKwargs}}
         @startingFolderId={{this.selectedFolder}}
         as |fileManager|
diff --git a/lib/osf-components/addon/components/addons-service/file-manager/component.ts b/lib/osf-components/addon/components/addons-service/file-manager/component.ts
@@ -1,3 +1,4 @@
+import { assert } from '@ember/debug';
 import { action } from '@ember/object';
 import { inject as service } from '@ember/service';
 import { waitFor } from '@ember/test-waiters';
@@ -8,13 +9,14 @@ import { taskFor } from 'ember-concurrency-ts';
 import IntlService from 'ember-intl/services/intl';
 import Toast from 'ember-toastr/services/toast';
 
-import { OperationKwargs } from 'ember-osf-web/models/configured-addon';
-import { Item, ListItemsResult } from 'ember-osf-web/models/addon-operation-invocation';
+import { Item, ListItemsResult, OperationKwargs } from 'ember-osf-web/models/addon-operation-invocation';
+import AuthorizedAccountModel from 'ember-osf-web/models/authorized-account';
 import ConfiguredAddonModel from 'ember-osf-web/models/configured-addon';
 import captureException, { getApiErrorMessage } from 'ember-osf-web/utils/capture-exception';
 
 interface Args {
-    configuredAddon: ConfiguredAddonModel;
+    configuredAddon?: ConfiguredAddonModel;
+    authorizedAccount?: AuthorizedAccountModel;
     defaultKwargs?: OperationKwargs;
     startingFolderId: string;
 }
@@ -23,6 +25,8 @@ export default class FileManager extends Component<Args> {
     @service intl!: IntlService;
     @service toast!: Toast;
 
+    @tracked operationInvocableModel!: ConfiguredAddonModel | AuthorizedAccountModel;
+
     @tracked currentPath: Item[] = [];
     @tracked currentItems: Item[] = [];
     @tracked currentFolderId?: string;
@@ -33,17 +37,19 @@ export default class FileManager extends Component<Args> {
     private lastInvocation: any = null;
 
     get isLoading() {
-        return taskFor(this.args.configuredAddon.getFolderItems).isRunning ||
+        return taskFor(this.operationInvocableModel.getFolderItems).isRunning ||
             taskFor(this.getStartingFolder).isRunning;
     }
 
     get isError() {
-        return taskFor(this.args.configuredAddon.getFolderItems).lastPerformed?.error ||
+        return taskFor(this.operationInvocableModel.getFolderItems).lastPerformed?.error ||
             taskFor(this.getStartingFolder).lastPerformed?.error;
     }
 
     constructor(owner: unknown, args: Args) {
         super(owner, args);
+        assert('Must provide a configuredAddon or authorizedAccount', args.configuredAddon || args.authorizedAccount);
+        this.operationInvocableModel = (args.configuredAddon || args.authorizedAccount)!;
         taskFor(this.initialize).perform();
     }
 
@@ -85,10 +91,10 @@ export default class FileManager extends Component<Args> {
     @task
     @waitFor
     async getStartingFolder() {
-        const { startingFolderId, configuredAddon } = this.args;
+        const { startingFolderId } = this.args;
         try {
             if (startingFolderId) {
-                const invocation = await taskFor(configuredAddon.getItemInfo).perform(startingFolderId);
+                const invocation = await taskFor(this.operationInvocableModel.getItemInfo).perform(startingFolderId);
                 const result = invocation.operationResult as Item;
                 this.currentFolderId = result.itemId;
                 this.currentPath = result.itemPath ? [...result.itemPath] : [];
@@ -109,7 +115,7 @@ export default class FileManager extends Component<Args> {
         kwargs.pageCursor = this.cursor;
         try {
             const getFolderArgs = !this.currentFolderId ? {} : kwargs;
-            const invocation = await taskFor(this.args.configuredAddon.getFolderItems).perform(getFolderArgs);
+            const invocation = await taskFor(this.operationInvocableModel.getFolderItems).perform(getFolderArgs);
             this.lastInvocation = invocation;
             const operationResult = invocation.operationResult as ListItemsResult;
             if (!this.currentFolderId) {
diff --git a/lib/osf-components/addon/components/addons-service/manager/component.ts b/lib/osf-components/addon/components/addons-service/manager/component.ts
@@ -222,12 +222,8 @@ export default class AddonsServiceManagerComponent extends Component<Args> {
         return false;
     }
 
-    @task
-    @waitFor
-    async confirmAccountSetup(account: AllAuthorizedAccountTypes) {
-        if (this.selectedProvider && this.selectedAccount) {
-            this.selectedConfiguration = await taskFor(this.selectedProvider.createConfiguredAddon).perform(account);
-        }
+    @action
+    confirmAccountSetup() {
         this.pageMode = PageMode.CONFIGURE;
     }
 
@@ -242,8 +238,12 @@ export default class AddonsServiceManagerComponent extends Component<Args> {
 
     @task
     @waitFor
-    async saveConfiguration(args: ConfiguredAddonEditableAttrs) {
+    async saveOrCreateConfiguration(args: ConfiguredAddonEditableAttrs) {
         try {
+            if (!this.selectedConfiguration && this.selectedProvider && this.selectedAccount) {
+                this.selectedConfiguration = await taskFor(this.selectedProvider.createConfiguredAddon)
+                    .perform(this.selectedAccount);
+            }
             if (this.selectedConfiguration && this.selectedConfiguration instanceof ConfiguredStorageAddonModel) {
                 this.selectedConfiguration.rootFolder = (args as ConfiguredAddonEditableAttrs).rootFolder;
                 this.selectedConfiguration.displayName = args.displayName;
diff --git a/lib/osf-components/addon/components/addons-service/manager/template.hbs b/lib/osf-components/addon/components/addons-service/manager/template.hbs
@@ -25,7 +25,7 @@
     connectAccount=this.connectAccount
     confirmAccountSetup=this.confirmAccountSetup
     cancelSetup=this.cancelSetup
-    saveConfiguration=this.saveConfiguration
+    saveOrCreateConfiguration=this.saveOrCreateConfiguration
     pageMode=this.pageMode
     headingText=this.headingText
     removeConfiguredAddon=this.removeConfiguredAddon
diff --git a/mirage/views/addons.ts b/mirage/views/addons.ts
@@ -1,12 +1,11 @@
 import { HandlerContext, ModelInstance, NormalizedRequestAttrs, Request, Response, Schema } from 'ember-cli-mirage';
 import { timeout } from 'ember-concurrency';
 
-import { InvocationStatus, ItemType } from 'ember-osf-web/models/addon-operation-invocation';
+import { ConnectedOperationNames, InvocationStatus, ItemType } from 'ember-osf-web/models/addon-operation-invocation';
 import AuthorizedCitationAccountModel from 'ember-osf-web/models/authorized-citation-account';
 import AuthorizedComputingAccountModel from 'ember-osf-web/models/authorized-computing-account';
 import { AddonCredentialFields} from 'ember-osf-web/models/authorized-account';
 import AuthorizedStorageAccountModel from 'ember-osf-web/models/authorized-storage-account';
-import { ConnectedOperationNames } from 'ember-osf-web/models/configured-addon';
 import { CredentialsFormat } from 'ember-osf-web/models/external-service';
 import ExternalStorageServiceModel from 'ember-osf-web/models/external-storage-service';
 import ExternalCitationServiceModel from 'ember-osf-web/models/external-citation-service';
diff --git a/tests/integration/components/addons-service/configured-addon-edit/component-test.ts b/tests/integration/components/addons-service/configured-addon-edit/component-test.ts
@@ -38,7 +38,7 @@ module('Integration | Component | addons-service | configured-addon-edit', funct
 
         const mirageConfiguredAddon = server.create('configured-storage-addon', {
             displayName: 'My configured addon',
-            rootFolder: 'rooty',
+            rootFolder: undefined,
             externalStorageService: boxAddon,
             accountOwner: userRef,
             authorizedResource:  resourceRef,
@@ -56,25 +56,26 @@ module('Integration | Component | addons-service | configured-addon-edit', funct
 />
         `);
 
+        // Initial state when no folder is selected
         assert.dom('[data-test-display-name-input]').hasValue('My configured addon', 'Display name is poopulated');
-        assert.dom('[data-test-folder-path-option]').exists({ count: 1 }, 'Has root folder path option');
+        assert.dom('[data-test-go-to-root]').exists('Go to root button exists');
+        assert.dom('[data-test-folder-path-option]').doesNotExist('No folder option when at root');
         assert.dom('[data-test-folder-link]').exists({ count: 5 }, 'Root folder has 5 folders');
         assert.dom('[data-test-root-folder-option]').exists({ count: 5 }, 'Checkbox available for each folder option');
-        assert.dom('[data-test-root-folder-save]').isEnabled('Save button is enabled');
+        assert.dom('[data-test-root-folder-save]').isDisabled('Save button is disabled when no folder is selected');
 
         // updating and reseting the display name
         assert.dom('[data-test-display-name-error]').doesNotExist('No error message initially');
         await fillIn('[data-test-display-name-input]', '');
         assert.dom('[data-test-root-folder-save]').isDisabled('Save button is disabled');
         assert.dom('[data-test-display-name-error]').exists('Error message is shown when display name is empty');
         await fillIn('[data-test-display-name-input]', 'My configured addon');
-        assert.dom('[data-test-root-folder-save]').isEnabled('Save button is enabled after display name is set');
         assert.dom('[data-test-display-name-error]').doesNotExist('No error message after display name is set');
 
         // Navigate into a folder
         const folderLinks = this.element.querySelectorAll('[data-test-folder-link]');
         await click(folderLinks[0]);
-        assert.dom('[data-test-folder-path-option]').exists({ count: 2 }, 'Has root folder and subfolder path option');
+        assert.dom('[data-test-folder-path-option]').exists({ count: 1 }, 'Has root option and first folderoption');
 
         // Navigate back to root
         const navButtons = this.element.querySelectorAll('[data-test-folder-path-option]');
@@ -89,7 +90,7 @@ module('Integration | Component | addons-service | configured-addon-edit', funct
         // Save
         await click('[data-test-root-folder-save]');
         const args = {
-            rootFolder: 'rooty-1',
+            rootFolder: 'root-1-1',
             displayName: 'My configured addon',
         };
 
