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

Pidp 1002/1098 add user notification for reasons cards are inaccessible #611

Open
wants to merge 19 commits into
base: develop
Choose a base branch
from

Conversation

AdityaMantripragada
Copy link
Collaborator

No description provided.

@AdityaMantripragada AdityaMantripragada added the Ready For Review PR is code-complete (or very close) and only needs some review and/or manual testing label Oct 11, 2024
this.heading === 'Driver Fitness Practitioner Portal'
) {
this.toastService.openInfoToast(
'Insufficient college licensing to request enrolment.',
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can keep these error messages in constants and use them here.

this.toastService.openInfoToast(
'Insufficient college licensing to request enrolment.',
'close',
{ duration: 100000, panelClass: 'close-icon' },
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as previous. duration can be kept in constants.

Copy link
Collaborator

@kakarlavinodkumar kakarlavinodkumar left a comment

Choose a reason for hiding this comment

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

functionality seems working fine.

{ duration: Constants.dialogDuration, panelClass: 'close-icon' },
);
} else {
this.toastService.closeToast();
Copy link
Collaborator

Choose a reason for hiding this comment

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

In which case it will be called ?

Copy link
Collaborator

@Paahn Paahn left a comment

Choose a reason for hiding this comment

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

I am a bit confused by the scoring and the description on this ticket. I understand that the idea of this ticket was to provide more information to the user however the notification messages are not entirely correct. We need to perform certain calculations to inform the user more accurately of why they don't have access to a card. Luckily we already perform these calculations in the the backend in ProfileStatus.Model, so we could inform the frontend of the appropriate message to display.

@AdityaMantripragada AdityaMantripragada added DO NOT MERGE Definitely don't merge this PR! and removed Ready For Review PR is code-complete (or very close) and only needs some review and/or manual testing labels Nov 13, 2024
@AdityaMantripragada AdityaMantripragada changed the title Pidp 1002 add user notification for reasons cards are inaccessible Pidp 1002/1098 add user notification for reasons cards are inaccessible Nov 18, 2024
@AdityaMantripragada AdityaMantripragada added Ready For Review PR is code-complete (or very close) and only needs some review and/or manual testing and removed DO NOT MERGE Definitely don't merge this PR! labels Nov 18, 2024
…-1002-Add-user-notification-for-reasons-cards-are-inaccessible-
@@ -251,6 +264,7 @@ public class PrescriptionRefillEformsSection : ProfileSection
{
internal override string SectionName => "prescriptionRefillEforms";
public override string[] KeyWords => ["pharmacists", "rx"];
public override string ErrorReason => "Incorrect credential type being used to request enrolment.";
Copy link
Collaborator

Choose a reason for hiding this comment

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

As the same value is being used in multiple places can we define in constants and re-use in multiple places.

Copy link

sonarqubecloud bot commented Dec 4, 2024

@Paahn Paahn self-requested a review January 3, 2025 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review PR is code-complete (or very close) and only needs some review and/or manual testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants