Skip to content
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.

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

BitstreamAuthorizationsComponent: update selector and comment to correctly describe component's purpose #3619

Merged
merged 2 commits into from
Nov 7, 2024

Conversation

saschaszott
Copy link
Contributor

Description

Trivial RP that fixes an incorrect comment in BitstreamAuthorizationsComponent.

@saschaszott saschaszott changed the title update comment to correctly describe component's purpose update selector and comment to correctly describe component's purpose Nov 7, 2024
@saschaszott saschaszott changed the title update selector and comment to correctly describe component's purpose BitstreamAuthorizationsComponent: update selector and comment to correctly describe component's purpose Nov 7, 2024
@tdonohue tdonohue added bug 1 APPROVAL pull request only requires a single approval to merge port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release labels Nov 7, 2024
Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

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

👍 Thanks @saschaszott ! This is definitely correct. I was initially confused by the selector change, but I looked into it & it's correct too. More info on that in the comment below (for any other developers who stumble on this PR).

@@ -19,7 +19,7 @@ import { DSpaceObject } from '../../core/shared/dspace-object.model';
import { ResourcePoliciesComponent } from '../../shared/resource-policies/resource-policies.component';

@Component({
selector: 'ds-collection-authorizations',
selector: 'ds-bitstream-authorizations',
Copy link
Member

Choose a reason for hiding this comment

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

This is very odd that this component has been using the wrong selector for so long (it seems to also be like this in 7.x and 8.x). Especially since we have a separate component that is also using `ds-collection-authorizations : https://github.com/DSpace/dspace-angular/blob/main/src/app/collection-page/edit-collection-page/collection-authorizations/collection-authorizations.component.ts#L18

This is definitely a bug. I was surprised it's not been found yet, but it seems that both of these components are only referenced via routes (e.g. this BitstreamAuthorizationComponent is loaded via bitstream-page-routes.ts). So, that's why we didn't notice this invalid selector earlier...the selector isn't being used right now.

@tdonohue tdonohue added this to the 9.0 milestone Nov 7, 2024
@tdonohue tdonohue merged commit 1ccdec2 into DSpace:main Nov 7, 2024
13 checks passed
@dspace-bot
Copy link
Contributor

@dspace-bot
Copy link
Contributor

@tdonohue tdonohue removed port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release labels Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 APPROVAL pull request only requires a single approval to merge bug
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants