Manage Subscriptions interface by atarix83 · Pull Request #2017 · DSpace/dspace-angular

atarix83 · 2022-12-30T08:41:57Z

References

Fixes Manage Subscriptions #803
Requires Subscription feature rest DSpace#8620

Description

This PR provides the opportunity to manage subscriptions by UI, in particular :

to subscribe/unsubscribe on the community/collection/item page itself
to show an overview of active subscriptions with the ability to remove them

Instructions for Reviewers

To test this PR you should run the UI against this REST PR.
Navigate to a DSO (community, collection or items) and try to subscribe to it. You can try to change the subscription option once you have alredy subscribed to the DSO.
Navigate to the subscriptions overview page to manage all the active subscriptions created.

List of changes in this PR:

Added to each DSO page a button to subscribe to itself
Created a modal to allow to subscribe daily/weekly/monthly updates
Created a new entry to the user menu to access the subscriptions overview page
Created a page to see the overview of user subscriptions and manage them

Include guidance for how to test or review your PR. This may include: steps to reproduce a bug, screenshots or description of a new feature, or reasons behind specific changes.

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
My PR passes ESLint validation using yarn lint
My PR doesn't introduce circular dependencies (verified via yarn check-circ-deps)
My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
If my PR fixes an issue ticket, I've linked them together.

# Conflicts: # src/app/shared/shared.module.ts

github-actions · 2023-01-10T17:46:20Z

Hi @atarix83,
Conflicts have been detected against the base branch.
Please resolve these conflicts as soon as you can. Thanks!

This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

# Conflicts: # src/app/shared/shared.module.ts

mwoodiupui · 2023-01-19T21:05:31Z

I am not seeing my Community and Collection subscriptions on the subscriptions overview page. Item subscriptions do appear. When I navigate to a Community or Collection to which I have subscribed, the subscriptions dialog shows the subscription as expected. I see the rows I would expect in the subscription and subscription_parameter tables.

tdonohue

@atarix83 : I tested & reviewed this today. The basics work, but I found quite a few small bugs.

A user's "Subscription" page doesn't automatically refresh when new subscriptions are added. How to reproduce:
- Login as a user. Visit the "Subscription" page. It should be empty initially.
- Now, browse to a Community/Collection or Item and subscribe to it.
- Go back to your "Subscription" page. It will still be empty.
- Click reload in your browser window. Now, you'll finally see the subscription.
- (This same issue occurs even if you have subscriptions already and add a new one. The new one won't appear until you reload your browser)
The Subscription popup modal lists "daily, monthly, weekly" (in that order). Ideally, it should say "Daily, Weekly, Monthly". The options should be capitalized, and "Weekly" should appear before "Monthly" as it's a shorter time period.
It's possible to add an empty frequency subscription by doing the following:
- Create an existing subscription.
- Go back to that same object and click "Subscription" again. This edits the existing subscription.
- Unselect all of the content options. Click Submit.
- The subscription is saved, but it now has no frequency. This shouldn't be valid, as a frequency currently seems to be required. Instead, the subscription should be deleted.
When looking at your Subscriptions (Under the user menu), they always have an empty Subject, making them impossible to tell apart. Clicking delete also shows an empty name. Here's a screenshot:
The Subscription modal popup should include the name of the object you are subscribing too. Otherwise, it's difficult to manage subscriptions from the User menu. Clicking "Edit" action in that list of subscriptions displays a popup that doesn't say which subscription you are editing.

src/app/shared/dso-page/dso-page-subscription-button/dso-page-subscription-button.component.ts

tdonohue · 2023-02-01T16:26:08Z

...app/shared/dso-page/dso-page-subscription-button/dso-page-subscription-button.component.html

@@ -0,0 +1,6 @@
+<button *ngIf="isAuthorized$ | async" data-test="subscription-button"
+        (click)="openSubscriptionModal()"
+        [ngbTooltip]="'item.page.subscriptions.tooltip' | translate"


This button is failing accessibility tests because it has no title or aria-label. Please add one or both. They can simply use the same i18n key.

tdonohue · 2023-02-01T16:27:06Z

src/app/shared/subscriptions/models/subscription.resource-type.ts

+import { ResourceType } from '../../../core/shared/resource-type';
+
+/**
+ * The resource type for Group


This comment is obviously wrong. This is a for a Subscription.

src/app/shared/subscriptions/subscription-modal/subscription-modal.component.ts

src/app/shared/subscriptions/subscription-view/subscription-view.component.ts

src/app/shared/subscriptions/subscriptions-data.service.ts

tdonohue · 2023-02-01T16:38:42Z

src/app/subscriptions-page/subscriptions-page.component.ts

+  templateUrl: './subscriptions-page.component.html',
+  styleUrls: ['./subscriptions-page.component.scss']
+})
+export class SubscriptionsPageComponent implements OnInit, OnDestroy {


Again, please add description or typedocs to describe where this component is used

tdonohue · 2023-02-01T16:41:58Z

src/assets/i18n/en.json5


  "item.page.claim.tooltip": "Claim this item as profile",

+  "item.page.subscriptions.tooltip": "Subscribe",


I find it odd that many of the subscription i18n tags start with "item.page" when they are obviously used for Communities, Collections and Items. These same keys are used for all DSOs.

Maybe these should just be named subscriptions.tooltip", etc (drop the entire "item.page" par, and move these down with the other subscriptions.*` keys)?

I don't think these should start with "item.page" as that implies these are only used for items

github-actions · 2023-02-02T16:35:41Z

Hi @atarix83,
Conflicts have been detected against the base branch.
Please resolve these conflicts as soon as you can. Thanks!

This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

davide-negretti · 2023-02-03T23:03:49Z

Hi @tdonohue, I made the requested changes

A user's "Subscription" page doesn't automatically refresh when new subscriptions are added

Fixed (the page previously used the cached values)

The Subscription popup modal lists "daily, monthly, weekly" (in that order)

Order fixed

It's possible to add an empty frequency subscription

I disabled the "Submit" button when no frequencies are selected. The button is also disabled if the user haven't changed the selected frequencies.

When looking at your Subscriptions (Under the user menu), they always have an empty Subject, making them impossible to tell apart. Clicking delete also shows an empty name.

Fixed (some parameters have been renamed on back-end)

The Subscription modal popup should include the name of the object you are subscribing too. Otherwise, it's difficult to manage subscriptions from the User menu. Clicking "Edit" action in that list of subscriptions displays a popup that doesn't say which subscription you are editing.

The name of the object is now showed in the modal

Requested changes to the code (comments and specs) are still in progress

…ency is missing

atarix83 · 2023-02-08T17:37:11Z

@tdonohue

Regarding this

As also noted on that screenshot, if it's easy to do so, I'd also recommend adding a "Delete" button to that popup. Currently, if you uncheck all options, the "Submit" button disables... this makes it appear that you cannot unsubscribe (obviously though, it is possible to unsubscribe from the user's "Subscription" menu though). This is a minor usability issue, so we could leave it as is for now, if it's too complex.

I think the edit modal should be intended only for changing the preferences, indeed we've added the validation to check if at least one option is selected. To complete unsubscribe the delete button should be use instead. Now the functionalities are quite separated and could be difficult to refactor it
Maybe we can simply add an info box message in the subscription page with a small explanation. Do you agree ?

tdonohue · 2023-02-08T18:23:18Z

@atarix83 : It's fine to add a note in that box which says something like:

To remove this subscription, please visit the "Subscriptions" page under your user profile

davide-negretti · 2023-02-08T18:58:11Z

Hi @tdonohue, I made the remaining changes:

missing tests
delete instructions
badge component
frequency order

tdonohue

Thanks @davide-negretti and @atarix83 : This is looking good now. Just one minor usability issue. The new "To remove this subscription..." message should ONLY be displayed when you are editing an existing subscription. Currently, it's always displayed on the popup... for instance this is what it looks like when creating a new subscription:

Obviously, this note makes no sense unless you are editing an existing subscription. It doesn't need to be displayed when you are creating a new subscription.

Beyond this small issue, I'm basically +1 this PR (though I'll have to wait on fixes to the backend PR before I can merge it)

…unavailable items

davide-negretti · 2023-02-09T11:07:43Z

@tdonohue I fixed the issue with the delete message, and also the issue with deleted/unaccessible items

github-actions · 2023-02-10T12:52:06Z

Hi @atarix83,
Conflicts have been detected against the base branch.
Please resolve these conflicts as soon as you can. Thanks!

This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

tdonohue · 2023-02-10T15:41:06Z

@atarix83 or @davide-negretti : Could one of you rebase this against main? It now has merge conflicts... so I'm waiting to retest this until those are resolved. Thanks!

tdonohue

@davide-negretti and @atarix83 : Overall I'm nearly a +1 on this PR. It's looking great. But, I think my previous request about the "To remove this subscription..." message was misunderstood.

I was asking that this message only be displayed when there is an existing subscription that you are editing. So, here's a screenshot where I've clicked on the Subscription button on a Collection & I already have a subscription. So, in this scenario, I expect the message should be displayed (but it is not).

So, here's when I think it should be displayed vs not displayed:

Click on Subscribe button on a Collection/Community homepage which you are not currently subscribed to. In this scenario, you are creating a new subscription, so the "To remove this subscription.." should not be displayed, as you have no subscription.
Click on Subscribe button on a Collection/Community homepage which you are already currently subscribed to. In this scenario, your existing subscription options will appear, and you should see the "To remove this subscription..." message. (Because you will not be able to remove your subscription from this page.. instead you have to go to the "Subscriptions" page to remove it.)
Click on the "Edit" button on your "Subscriptions" page in the Profile. In this scenario, the "To remove this subscription.." message should likely not be displayed, as you are already on the "Subscriptions page. (That said, if it has to be displayed here, I'm OK with it. I just want the first two bullets to be fixed)

Once this message is displayed properly & this PR has it's merge conflicts resolved, then I'm +1 this feature. Thanks for all the hard work as this is looking good!

# Conflicts: # src/app/collection-page/collection-page.component.html # src/app/community-page/community-page.component.html # src/app/core/core.module.ts # src/app/core/data/feature-authorization/feature-id.ts # src/app/shared/shared.module.ts

davide-negretti · 2023-02-13T14:47:25Z

@tdonohue I fixed the issue with the delete messsage. Your initial request was clear but I made an error in the implementation.

tdonohue

👍 Thanks @davide-negretti and @atarix83 ! I've re-tested today, and this is now working properly. All prior feedback has been addressed.

rezartatis and others added 11 commits December 22, 2022 17:11

[CST-7757] Subscriptions porting (wip)

b775cd5

[CST-7757] Subscriptions porting (wip)

f15ca85

[CST-7757] Subscriptions porting (wip)

3ab552e

[CST-7757] Subscriptions porting (wip)

a6cd39c

Merge branch 'main' into CST-7757

af0b7e7

# Conflicts: # src/app/shared/shared.module.ts

[CST-7757] Refactoring code

72a42b0

[CST-7757] Fix lint errors

88e191e

[CST-7757] fix tests

88eefc6

[CST-7757] Add subscribe button to all dso pages

27d5097

[CST-7757] fix pagination

90a9238

[CST-7757] Rename type property to subscriptionType

710b116

atarix83 changed the title ~~Cst 7757~~ Manage Subscriptions interface Dec 30, 2022

corrad82-4s mentioned this pull request Dec 30, 2022

Subscription feature rest DSpace/DSpace#8620

Merged

8 tasks

tdonohue self-requested a review January 5, 2023 15:37

tdonohue assigned atarix83 Jan 5, 2023

tdonohue requested a review from mwoodiupui January 5, 2023 15:37

github-actions bot added the merge conflict label Jan 10, 2023

atarix83 added 2 commits January 16, 2023 09:19

Merge branch 'main' into CST-7757

1cc03f4

# Conflicts: # src/app/shared/shared.module.ts

[CST-7757] Fix merge with main

28351f2

github-actions bot removed the merge conflict label Jan 16, 2023

[CST-7757] renamed param

b09de30

tdonohue requested changes Feb 1, 2023

View reviewed changes

github-actions bot added the merge conflict label Feb 2, 2023

Davide Negretti added 3 commits February 6, 2023 10:23

[CST-7757] Remove subscription button from item pages

b1011ed

[CST-7757] Response parameters fixed; submit button disabled if frequ…

a4d94b0

…ency is missing

[CST-7757] D-W-M fixed

0f8d2d7

[CST-7757] subscriptions data service tests

6bfcc25

davide-negretti force-pushed the CST-7757 branch from f03b4d9 to 6bfcc25 Compare February 8, 2023 12:08

[CST-7757] subscription modal tests

c8e7ac3

Davide Negretti added 2 commits February 8, 2023 19:38

[CST-7757] modal fixes

325d78d

[CST-7757] modal fixes

27f541e

tdonohue self-requested a review February 8, 2023 18:59

tdonohue reviewed Feb 8, 2023

View reviewed changes

[CST-7757] Hide delete info for new subscriptions and fix issue with …

516b91f

…unavailable items

tdonohue added new feature 1 APPROVAL pull request only requires a single approval to merge labels Feb 9, 2023

tdonohue self-requested a review February 9, 2023 15:30

github-actions bot added the merge conflict label Feb 10, 2023

tdonohue requested changes Feb 10, 2023

View reviewed changes

Davide Negretti added 2 commits February 11, 2023 03:17

Merge branch 'main-gh4s' into CST-7757

d65c12a

# Conflicts: # src/app/collection-page/collection-page.component.html # src/app/community-page/community-page.component.html # src/app/core/core.module.ts # src/app/core/data/feature-authorization/feature-id.ts # src/app/shared/shared.module.ts

[CST-7757] Missing label

851fe78

github-actions bot removed the merge conflict label Feb 11, 2023

[CST-7757] Fix delete message

078bdd2

tdonohue self-requested a review February 13, 2023 15:23

tdonohue approved these changes Feb 13, 2023

View reviewed changes

tdonohue merged commit bd428d7 into DSpace:main Feb 13, 2023

tdonohue added this to the 7.5 milestone Feb 17, 2023

This was referenced Jul 20, 2023

Subscription button should be moved to the DSO menu #2384

Closed

Move subscription button to DSO edit menu #2385

Merged


		"item.page.claim.tooltip": "Claim this item as profile",

		"item.page.subscriptions.tooltip": "Subscribe",

Conversation

atarix83 commented Dec 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

References

Description

Instructions for Reviewers

Checklist

Uh oh!

github-actions bot commented Jan 10, 2023

Uh oh!

mwoodiupui commented Jan 19, 2023

Uh oh!

tdonohue left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tdonohue Feb 1, 2023

Choose a reason for hiding this comment

Uh oh!

tdonohue Feb 1, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tdonohue Feb 1, 2023

Choose a reason for hiding this comment

Uh oh!

tdonohue Feb 1, 2023

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 2, 2023

Uh oh!

davide-negretti commented Feb 3, 2023

Uh oh!

atarix83 commented Feb 8, 2023

Uh oh!

tdonohue commented Feb 8, 2023

Uh oh!

davide-negretti commented Feb 8, 2023

Uh oh!

tdonohue left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davide-negretti commented Feb 9, 2023

Uh oh!

github-actions bot commented Feb 10, 2023

Uh oh!

tdonohue commented Feb 10, 2023

Uh oh!

tdonohue left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davide-negretti commented Feb 13, 2023

Uh oh!

tdonohue left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

atarix83 commented Dec 30, 2022 •

edited

Loading

tdonohue left a comment •

edited

Loading

tdonohue left a comment •

edited

Loading