Skip to content

Conversation

@nsemets
Copy link
Collaborator

@nsemets nsemets commented Nov 12, 2025

  • Ticket: [ENG-9749]
  • Feature flag: n/a

Summary of Changes

  1. Fixed endless loading for addons.

@nsemets nsemets marked this pull request as ready for review November 12, 2025 10:41
@nsemets nsemets requested a review from brianjgeiger November 12, 2025 10:41
@nsemets nsemets changed the base branch from develop to feature/pbs-25.04 November 12, 2025 10:41
@coveralls
Copy link
Collaborator

Coverage Status

coverage: 55.46% (+1.1%) from 54.331%
when pulling 5aa204a on nsemets:fix/ENG-9749
into b2bdce5 on CenterForOpenScience:develop.

Copy link
Contributor

@brianjgeiger brianjgeiger left a comment

Choose a reason for hiding this comment

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

A couple of comments regarding drag and drop that I don't think are relevant because I'm pretty sure you can't drag and drop across providers in the current interface. Otherwise looks good.

[value]="nodes()"
[draggableNodes]="true"
[droppableNodes]="true"
[draggableNodes]="!hasViewOnly() && supportUpload()"
Copy link
Contributor

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

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

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

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.

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

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.

@brianjgeiger brianjgeiger merged commit 16cc0bf into CenterForOpenScience:feature/pbs-25.04 Nov 12, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants