Skip to content

[ENG-9749] Navigation Project Addons page → Linked Services (from dropdown box) causes an infinite loading loop. #756

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 12, 2025
Merged

[ENG-9749] Navigation Project Addons page → Linked Services (from dropdown box) causes an infinite loading loop. #756

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1dbfba8
fix(addons): fixed endless loading
nsemets Nov 12, 2025
5aa204a
fix(addons): fixed unit tests
nsemets Nov 12, 2025
69aacb5
fix(files-tree): fixed drag and drop
nsemets Nov 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions ...p/features/project/project-addons/components/configure-addon/configure-addon.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,9 @@ export class ConfigureAddonComponent implements OnInit {
});

readonly supportedResourceTypes = computed(() => {
if (this.linkAddons().length && this.addonTypeString() === AddonType.LINK) {
const addon = this.linkAddons().find((a) => this.addon()?.externalServiceName === a.externalServiceName);
const linkAddons = this.linkAddons();
if (linkAddons?.length && this.addonTypeString() === AddonType.LINK) {
const addon = linkAddons.find((a) => this.addon()?.externalServiceName === a.externalServiceName);
return addon?.supportedResourceTypes || [];
}
return [];
Expand Down
5 changes: 3 additions & 2 deletions src/app/features/project/project-addons/project-addons.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,9 @@ export class ProjectAddonsComponent implements OnInit {
filteredAddonCards = computed((): AddonCardModel[] => {
const searchValue = this.searchValue().toLowerCase();
const configuredAddons = this.allConfiguredAddonsForCheck();
const addons = this.currentAddonsState() ?? [];

const addonCards = this.currentAddonsState()
const addonCards = addons
.filter(
(card) =>
card.externalServiceName.toLowerCase().includes(searchValue) ||
Expand Down Expand Up @@ -300,7 +301,7 @@ export class ProjectAddonsComponent implements OnInit {
const addons = this.currentAddonsState();
const isLoading = this.currentAddonsLoading();

if (!addons?.length && !isLoading) {
if (!addons && !isLoading) {
action();
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/app/features/settings/settings-addons/settings-addons.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,8 @@ export class SettingsAddonsComponent implements OnInit {

readonly filteredAddonCards = computed(() => {
const searchValue = this.searchValue().toLowerCase();
const filteredAddons = this.currentAddonsState().filter(
const addons = this.currentAddonsState() ?? [];
const filteredAddons = addons.filter(
(card) =>
card.externalServiceName.toLowerCase().includes(searchValue) ||
card.displayName.toLowerCase().includes(searchValue)
Expand Down Expand Up @@ -259,7 +260,7 @@ export class SettingsAddonsComponent implements OnInit {
const action = this.currentAction();
const addons = this.currentAddonsState();

if (!addons?.length) {
if (!addons) {
action();
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/app/shared/components/files-tree/files-tree.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
<div class="files-table flex">
<p-tree
[value]="nodes()"
[draggableNodes]="true"
[droppableNodes]="true"
[draggableNodes]="!hasViewOnly() && supportUpload()"
Copy link
Contributor

@brianjgeiger brianjgeiger Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If supportUpload menas that the current storage provider can be uploaded to, I don't think that's correct. You should be able to drag from any provider you can read from, but you can only drop on providers that support uploads.

Copy link
Contributor

@brianjgeiger brianjgeiger Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As below, if you can't drag and drop between providers, then this may not matter and can remain as it is.

Copy link
Collaborator Author

@nsemets nsemets Nov 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are conditions for supportUpload(): this.supportedFeatures()[this.provider()]?.includes(SupportedFeature.AddUpdateFiles) && this.canEdit() && !this.isRegistration(). Also, it's a drag-and-drop action within the same provider, not between different providers.

Copy link
Contributor

@brianjgeiger brianjgeiger Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, should be fine then.

[droppableNodes]="!hasViewOnly() && supportUpload()"
Copy link
Contributor

@brianjgeiger brianjgeiger Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also note that some providers (well, Github, but potentially others) have a "Can copy into" flag disabled from the addons service capabilities list. This means that you can't copy into those providers. This may not matter for drag and drop, because I don't think you can drag to another provider, but if you can, then this would be disabled in some other cases.

(onScrollIndexChange)="onScrollIndexChange($event)"
[scrollHeight]="scrollHeight()"
[virtualScroll]="true"
Expand Down
12 changes: 6 additions & 6 deletions src/app/shared/stores/addons/addons.models.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import { OperationInvocation } from '@osf/shared/models/addons/operation-invocat
import { AsyncStateModel } from '@osf/shared/models/store/async-state.model';

export interface AddonsStateModel {
storageAddons: AsyncStateModel<AddonModel[]>;
citationAddons: AsyncStateModel<AddonModel[]>;
linkAddons: AsyncStateModel<AddonModel[]>;
storageAddons: AsyncStateModel<AddonModel[] | null>;
citationAddons: AsyncStateModel<AddonModel[] | null>;
linkAddons: AsyncStateModel<AddonModel[] | null>;
authorizedStorageAddons: AsyncStateModel<AuthorizedAccountModel[]>;
authorizedCitationAddons: AsyncStateModel<AuthorizedAccountModel[]>;
authorizedLinkAddons: AsyncStateModel<AuthorizedAccountModel[]>;
Expand All @@ -30,17 +30,17 @@ export interface AddonsStateModel {

export const ADDONS_DEFAULTS: AddonsStateModel = {
storageAddons: {
data: [],
data: null,
isLoading: false,
error: null,
},
citationAddons: {
data: [],
data: null,
isLoading: false,
error: null,
},
linkAddons: {
data: [],
data: null,
isLoading: false,
error: null,
},
Expand Down
8 changes: 4 additions & 4 deletions src/app/shared/stores/addons/addons.selectors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ import { AddonsState } from './addons.state';

export class AddonsSelectors {
@Selector([AddonsState])
static getStorageAddons(state: AddonsStateModel): AddonModel[] {
static getStorageAddons(state: AddonsStateModel): AddonModel[] | null {
return state.storageAddons.data;
}

static getStorageAddon(id: string): (state: AddonsStateModel) => AddonModel | null {
return createSelector([AddonsState], (state: AddonsStateModel): AddonModel | null => {
return state.storageAddons.data.find((addon: AddonModel) => addon.id === id) || null;
return state.storageAddons.data?.find((addon: AddonModel) => addon.id === id) || null;
});
}

Expand All @@ -32,7 +32,7 @@ export class AddonsSelectors {
}

@Selector([AddonsState])
static getCitationAddons(state: AddonsStateModel): AddonModel[] {
static getCitationAddons(state: AddonsStateModel): AddonModel[] | null {
return state.citationAddons.data;
}

Expand All @@ -42,7 +42,7 @@ export class AddonsSelectors {
}

@Selector([AddonsState])
static getLinkAddons(state: AddonsStateModel): AddonModel[] {
static getLinkAddons(state: AddonsStateModel): AddonModel[] | null {
return state.linkAddons.data;
}

Expand Down
4 changes: 2 additions & 2 deletions src/app/shared/stores/addons/addons.state.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe('State: Addons', () => {
it('should fetch storage addons and update state and selector output', inject(
[HttpTestingController],
(httpMock: HttpTestingController) => {
let result: any[] = [];
let result: any[] | null = [];
store.dispatch(new GetStorageAddons()).subscribe(() => {
result = store.selectSnapshot(AddonsSelectors.getStorageAddons);
});
Expand Down Expand Up @@ -106,7 +106,7 @@ describe('State: Addons', () => {
req.flush({ message: 'Internal Server Error' }, { status: 500, statusText: 'Server Error' });

expect(result).toEqual({
data: [],
data: null,
error: 'Http failure response for http://addons.localhost:8000/external-storage-services: 500 Server Error',
isLoading: false,
isSubmitting: false,
Expand Down