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.

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

Jump to bottom

feat: As a customer, I want to view and update my preferred notification channels #17843

Open
wants to merge 23 commits into
base: develop-6.7.x
Choose a base branch
from
Open

feat: As a customer, I want to view and update my preferred notification channels #17843

wants to merge 23 commits into from

Conversation

Melody-zhou-512
Copy link
Contributor

@Melody-zhou-512 Melody-zhou-512 commented Sep 19, 2023 •
edited
Loading

As a customer, I want to view and update my preferred notification channels.
JIRA task: https://jira.tools.sap/browse/CXSPA-4468
The e2e test JIRA task: https://jira.tools.sap/browse/CXSPA-4475

@Melody-zhou-512
New Notification Preference Component
73d66d8 
As a customer, I want to view and update my preferred notification channels
@Melody-zhou-512 Melody-zhou-512 requested a review from a team as a code owner September 19, 2023 06:39
@github-actions github-actions bot marked this pull request as draft September 19, 2023 06:39
@Melody-zhou-512 Melody-zhou-512 changed the title New Notification Preference Component feature/CXSPA-4468:As a customer, I want to view and update my preferred notification channels Sep 19, 2023
@i53577
Copy link
Contributor

i53577 commented Sep 19, 2023

please use yarn prettier:fix to format the code

@Melody-zhou-512
Changes from code review comments
eda046a 
format the code and other changes from code review comments
@i53577 i53577 changed the title feature/CXSPA-4468:As a customer, I want to view and update my preferred notification channels feat: As a customer, I want to view and update my preferred notification channels Sep 19, 2023
i53577
i53577 previously approved these changes Sep 20, 2023
View reviewed changes
Copy link
Contributor

@i53577 i53577 left a comment

Choose a reason for hiding this comment

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

LGTM
please mark the PR as ready for review to trigger more tests

@Melody-zhou-512 Melody-zhou-512 marked this pull request as ready for review September 20, 2023 08:23
@cypress
Copy link

cypress bot commented Sep 20, 2023 •
edited
Loading

4 flaky tests on run #42051 ↗︎

0 119 2 0 Flakiness 4

Details:

Merge 5260493 into e4739e8...
Project: spartacus Commit: 269c1738c9 ℹ️
Status: Passed Duration: 04:42 💡
Started: Nov 30, 2023 9:28 AM Ended: Nov 30, 2023 9:33 AM
Flakiness  regression/asm/asm.deeplink.core-e2e.cy.ts • 1 flaky test • B2C

View Output Video

Test Artifacts
Assisted Service Module > Customer Support Agent - Emulation&deeplink > should emulate customer and navigate to saved cart with deeplink after agent login (CXSPA-3263) Test Replay Screenshots Video
Flakiness  ssr/pages.core-e2e.cy.ts • 3 flaky tests • SSR

View Output Video

Test Artifacts
SSR > should render homepage Test Replay Screenshots Video
SSR > should render PLP Test Replay Screenshots Video
SSR > should render PDP Test Replay Screenshots Video

Review all test suite changes for PR #17843 ↗︎

Zeyber
Zeyber requested changes Sep 20, 2023
View reviewed changes
Copy link
Contributor

@Zeyber Zeyber left a comment

Choose a reason for hiding this comment

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

Are we missing e2e tests?

...ms-components/myaccount/new-notification-preference/new-notification-preference.component.ts Outdated
protected preferences: NotificationPreference[] = [];

constructor(
private notificationPreferenceService: UserNotificationPreferenceService
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use protected instead of private to make this easier for users to extend.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

modified as suggestion.Only after CDC/CDP changing cms config and introduce `newNotificationPreference' in the future, it will be displayed on SPA UI. That's why there is no e2e test. But we already have a ticket to track it.
https://jira.tools.sap/browse/CXSPA-4475

...b/cms-components/myaccount/new-notification-preference/new-notification-preference.module.ts Outdated Show resolved Hide resolved
...-components/myaccount/new-notification-preference/new-notification-preference.component.html Outdated Show resolved Hide resolved
@Melody-zhou-512
Adapt codes as comments
ffd2a17 
Adapt codes as comments
@github-actions github-actions bot marked this pull request as draft September 22, 2023 03:06
@Melody-zhou-512 Melody-zhou-512 marked this pull request as ready for review September 22, 2023 04:17
@Melody-zhou-512 Melody-zhou-512 requested a review from a team November 22, 2023 08:38
@Melody-zhou-512 Melody-zhou-512 requested review from a team as code owners November 22, 2023 08:38
@github-actions github-actions bot marked this pull request as draft November 24, 2023 08:07
@Melody-zhou-512 Melody-zhou-512 marked this pull request as ready for review November 24, 2023 10:17
@github-actions github-actions bot marked this pull request as draft November 24, 2023 10:20
@Melody-zhou-512 Melody-zhou-512 marked this pull request as ready for review November 27, 2023 05:25
@github-actions github-actions bot marked this pull request as draft November 27, 2023 05:28
@Melody-zhou-512 Melody-zhou-512 marked this pull request as ready for review November 27, 2023 05:29
@github-actions github-actions bot marked this pull request as draft November 27, 2023 05:31
@Melody-zhou-512
fix check issue
3e1b125 
fix check issue
@Melody-zhou-512 Melody-zhou-512 marked this pull request as ready for review November 27, 2023 08:43
@github-actions github-actions bot marked this pull request as draft November 27, 2023 08:57
@Melody-zhou-512 Melody-zhou-512 marked this pull request as ready for review November 28, 2023 05:29
@Melody-zhou-512
fix sonar issue
430e520 
fix sonar issue
@github-actions github-actions bot marked this pull request as draft November 28, 2023 05:46
@Melody-zhou-512 Melody-zhou-512 marked this pull request as ready for review November 28, 2023 05:47
@Melody-zhou-512
fix e2e test issue
996641f 
fix e2e test issue
@github-actions github-actions bot marked this pull request as draft November 28, 2023 06:30
@Melody-zhou-512 Melody-zhou-512 marked this pull request as ready for review November 28, 2023 06:41
@github-actions github-actions bot marked this pull request as draft November 30, 2023 09:16
@Melody-zhou-512 Melody-zhou-512 marked this pull request as ready for review November 30, 2023 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Zeyber Zeyber Zeyber requested changes

@i53577 i53577 i53577 left review comments

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Melody-zhou-512 @i53577 @Zeyber