-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat: fetch provider list on portal based on selected application #373
base: main
Are you sure you want to change the base?
Changes from 5 commits
1a341e6
a1dfd5c
bfb40f6
e4e9993
220abdb
ed29e08
ff6011d
1c14495
3fe2a09
3f6cb74
4fd8b8f
8389ddf
38a131e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -7,6 +7,8 @@ import { NotificationsService } from './notifications.service'; | |||||||||||||||||||||||||||||
import { Notification, NotificationResponse } from './notification.model'; | ||||||||||||||||||||||||||||||
import { ApplicationsService } from '../applications/applications.service'; | ||||||||||||||||||||||||||||||
import { ApplicationResponse } from '../applications/application.model'; | ||||||||||||||||||||||||||||||
import { ProvidersService } from '../providers/providers.service'; | ||||||||||||||||||||||||||||||
import { ProviderResponse } from '../providers/provider.model'; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
@Component({ | ||||||||||||||||||||||||||||||
selector: 'app-notifications', | ||||||||||||||||||||||||||||||
|
@@ -36,24 +38,21 @@ export class NotificationsComponent implements OnInit { | |||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
// for component | ||||||||||||||||||||||||||||||
channelTypes = Object.entries(ChannelType).map(([, value]) => ({ | ||||||||||||||||||||||||||||||
label: `${this.channelTypeMap[value].altText} - ${this.channelTypeMap[value].providerName}`, | ||||||||||||||||||||||||||||||
value, | ||||||||||||||||||||||||||||||
})); | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
deliveryStatuses = Object.entries(DeliveryStatus).map(([, value]) => ({ | ||||||||||||||||||||||||||||||
label: this.deliveryStatusMap[value].value, | ||||||||||||||||||||||||||||||
value, | ||||||||||||||||||||||||||||||
})); | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
applications = []; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
selectedChannelType = null; | ||||||||||||||||||||||||||||||
providers = []; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
selectedDeliveryStatus = null; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
selectedApplication = null; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
selectedProvider = null; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
selectedFromDate = null; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
selectedToDate = null; | ||||||||||||||||||||||||||||||
|
@@ -85,6 +84,7 @@ export class NotificationsComponent implements OnInit { | |||||||||||||||||||||||||||||
constructor( | ||||||||||||||||||||||||||||||
private notificationService: NotificationsService, | ||||||||||||||||||||||||||||||
private applicationService: ApplicationsService, | ||||||||||||||||||||||||||||||
private providerService: ProvidersService, | ||||||||||||||||||||||||||||||
private messageService: MessageService, | ||||||||||||||||||||||||||||||
private authService: AuthService, | ||||||||||||||||||||||||||||||
) {} | ||||||||||||||||||||||||||||||
|
@@ -177,6 +177,9 @@ export class NotificationsComponent implements OnInit { | |||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
// Now that applications are loaded, load notifications | ||||||||||||||||||||||||||||||
this.loadNotificationsLazy({ first: 0, rows: this.pageSize }); | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
// Load providers for selected application | ||||||||||||||||||||||||||||||
this.getProvidersForSelectedApplication(); | ||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
|
@@ -225,6 +228,89 @@ export class NotificationsComponent implements OnInit { | |||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
getProvidersForSelectedApplication() { | ||||||||||||||||||||||||||||||
// Set the query variables | ||||||||||||||||||||||||||||||
const variables = { | ||||||||||||||||||||||||||||||
filters: [], | ||||||||||||||||||||||||||||||
offset: 0, | ||||||||||||||||||||||||||||||
limit: 15, | ||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
if (!this.selectedApplication) { | ||||||||||||||||||||||||||||||
// Handle missing selected application | ||||||||||||||||||||||||||||||
return; | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
// Set query filters | ||||||||||||||||||||||||||||||
variables.filters.push({ | ||||||||||||||||||||||||||||||
field: 'applicationId', | ||||||||||||||||||||||||||||||
operator: 'eq', | ||||||||||||||||||||||||||||||
value: this.selectedApplication.toString(), | ||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
// Fetch the login token | ||||||||||||||||||||||||||||||
const loginToken = this.getJWTLoginToken(); | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
if (!loginToken) { | ||||||||||||||||||||||||||||||
// Handle missing token | ||||||||||||||||||||||||||||||
return; | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
Comment on lines
+271
to
+274
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance error handling for missing login token. The error handling for missing login token should follow the same pattern as other error cases in the component. if (!loginToken) {
- // Handle missing token
+ this.messageService.add({
+ key: 'tst',
+ severity: 'error',
+ summary: 'Error',
+ detail: 'Login token is missing. Please log in again.',
+ });
+ this.loading = false;
return;
} 📝 Committable suggestion
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handled in different PR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kshitij-k-osmosys Understood. Thanks for clarifying that the missing login token scenario is handled by logging out the user.
✏️ Learnings added
|
||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
// Fetch providers and handle errors | ||||||||||||||||||||||||||||||
this.providerService | ||||||||||||||||||||||||||||||
.getProviders(variables, loginToken) | ||||||||||||||||||||||||||||||
.pipe( | ||||||||||||||||||||||||||||||
// catchError operator to handle errors | ||||||||||||||||||||||||||||||
catchError((error) => { | ||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't have common code for this code block? Can add a TODO task for this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we create an issue? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kshitij-k-osmosys No need, directly create a task in pinestem for now in backlog. |
||||||||||||||||||||||||||||||
this.messageService.add({ | ||||||||||||||||||||||||||||||
key: 'tst', | ||||||||||||||||||||||||||||||
severity: 'error', | ||||||||||||||||||||||||||||||
summary: 'Error', | ||||||||||||||||||||||||||||||
detail: `There was an error while loading providers. Reason: ${error.message}`, | ||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||
return of(null); // Return null to indicate error | ||||||||||||||||||||||||||||||
}), | ||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||
.subscribe((providerResponse: ProviderResponse | null) => { | ||||||||||||||||||||||||||||||
if (providerResponse?.errors?.length) { | ||||||||||||||||||||||||||||||
const unauthorizedError = providerResponse.errors.find( | ||||||||||||||||||||||||||||||
(error) => error.message === 'Unauthorized', | ||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
this.providers = []; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
if (unauthorizedError) { | ||||||||||||||||||||||||||||||
this.messageService.add({ | ||||||||||||||||||||||||||||||
key: 'tst', | ||||||||||||||||||||||||||||||
severity: 'error', | ||||||||||||||||||||||||||||||
summary: 'Error', | ||||||||||||||||||||||||||||||
detail: 'Unauthorized access. Please log in again.', | ||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||
this.authService.logoutUser(); | ||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||
providerResponse.errors.forEach((error) => { | ||||||||||||||||||||||||||||||
this.messageService.add({ | ||||||||||||||||||||||||||||||
key: 'tst', | ||||||||||||||||||||||||||||||
severity: 'error', | ||||||||||||||||||||||||||||||
summary: 'Error', | ||||||||||||||||||||||||||||||
detail: `GraphQL Error - Get Providers: ${error.message}`, | ||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
} else if (providerResponse?.providers?.length) { | ||||||||||||||||||||||||||||||
// Fetch list of providers for dropdown | ||||||||||||||||||||||||||||||
this.providers = providerResponse.providers.map((obj) => ({ | ||||||||||||||||||||||||||||||
// Name to display and ID to return upon selection | ||||||||||||||||||||||||||||||
label: `${obj.name} - ${this.channelTypeMap[obj.channelType].altText}`, | ||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add null check for channelType mapping The code assumes that the provider's channelType will always exist in channelTypeMap. Add a null check to handle potential missing mappings. - label: `${obj.name} - ${this.channelTypeMap[obj.channelType].altText}`,
+ label: `${obj.name}${obj.channelType && this.channelTypeMap[obj.channelType]?.altText ? ` - ${this.channelTypeMap[obj.channelType].altText}` : ''}`, 📝 Committable suggestion
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed 4fd8b8f There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kshitij-k-osmosys Thank you for implementing the fix! Your solution with the fallback to 🔗 Analysis chain@kshitij-k-osmosys Thank you for implementing the fix! The code is now more robust in handling missing channel type mappings. 🏁 Scripts executedThe following scripts were executed for the analysis: Script:
Length of output: 1212 If you found this review helpful, would you consider giving us a shout-out on X? |
||||||||||||||||||||||||||||||
value: obj.providerId, | ||||||||||||||||||||||||||||||
})); | ||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||
this.providers = []; | ||||||||||||||||||||||||||||||
this.selectedProvider = null; | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
loadNotificationsLazy(event: LazyLoadEvent) { | ||||||||||||||||||||||||||||||
this.loading = true; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
|
@@ -257,29 +343,6 @@ export class NotificationsComponent implements OnInit { | |||||||||||||||||||||||||||||
value: this.selectedApplication.toString(), | ||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
if (this.selectedChannelType) { | ||||||||||||||||||||||||||||||
if (this.selectedChannelType === this.allPortalChannelTypes.UNKNOWN) { | ||||||||||||||||||||||||||||||
// Condition to filter all notifications with unknown channel type | ||||||||||||||||||||||||||||||
const existingChannelTypes = Object.keys(this.channelTypeMap).filter( | ||||||||||||||||||||||||||||||
(value) => value !== this.allPortalChannelTypes.UNKNOWN.toString(), | ||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||
existingChannelTypes.forEach((key) => { | ||||||||||||||||||||||||||||||
variables.filters.push({ | ||||||||||||||||||||||||||||||
field: 'channelType', | ||||||||||||||||||||||||||||||
operator: 'ne', | ||||||||||||||||||||||||||||||
value: key.toString(), | ||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||
// Default behavior when we are sorting on known channelType | ||||||||||||||||||||||||||||||
variables.filters.push({ | ||||||||||||||||||||||||||||||
field: 'channelType', | ||||||||||||||||||||||||||||||
operator: 'eq', | ||||||||||||||||||||||||||||||
value: this.selectedChannelType.toString(), | ||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
if (this.selectedDeliveryStatus) { | ||||||||||||||||||||||||||||||
variables.filters.push({ | ||||||||||||||||||||||||||||||
field: 'deliveryStatus', | ||||||||||||||||||||||||||||||
|
@@ -288,6 +351,14 @@ export class NotificationsComponent implements OnInit { | |||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
if (this.selectedProvider) { | ||||||||||||||||||||||||||||||
variables.filters.push({ | ||||||||||||||||||||||||||||||
field: 'providerId', | ||||||||||||||||||||||||||||||
operator: 'eq', | ||||||||||||||||||||||||||||||
value: this.selectedProvider.toString(), | ||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
if (this.selectedFromDate) { | ||||||||||||||||||||||||||||||
variables.filters.push({ | ||||||||||||||||||||||||||||||
field: 'createdOn', | ||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
import { GraphQLFormattedError } from 'graphql/error/GraphQLError'; | ||
|
||
export interface Provider { | ||
providerId: number; | ||
name: string; | ||
channelType: number; | ||
createdOn: Date; | ||
updatedOn: Date; | ||
status: number; | ||
} | ||
|
||
export interface ProviderResponse { | ||
providers: Provider[]; | ||
total: number; | ||
offset: number; | ||
limit: number; | ||
errors?: ReadonlyArray<GraphQLFormattedError>; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
import { Injectable } from '@angular/core'; | ||
import { Observable, catchError, map } from 'rxjs'; | ||
import { GraphqlService } from 'src/app/graphql/graphql.service'; | ||
import { GetProviders } from 'src/app/graphql/graphql.queries'; | ||
import { ApolloQueryResult } from '@apollo/client/core'; | ||
import { Provider, ProviderResponse } from './provider.model'; | ||
|
||
interface GetProvidersResponse { | ||
providers: { | ||
providers?: Provider[]; | ||
total?: number; | ||
offset?: number; | ||
limit?: number; | ||
}; | ||
} | ||
@Injectable({ | ||
providedIn: 'root', | ||
}) | ||
export class ProvidersService { | ||
constructor(private graphqlService: GraphqlService) {} | ||
|
||
getProviders(variables, inputToken): Observable<ProviderResponse> { | ||
return this.graphqlService.query(GetProviders, variables, inputToken).pipe( | ||
map((response: ApolloQueryResult<GetProvidersResponse>) => { | ||
const providerArray = response.data?.providers.providers; | ||
|
||
const providerResponseObject: ProviderResponse = { | ||
providers: providerArray, | ||
total: response.data?.providers.total, | ||
offset: response.data?.providers.offset, | ||
limit: response.data?.providers.limit, | ||
errors: response.errors || null, | ||
}; | ||
return providerResponseObject; | ||
}), | ||
catchError((error) => { | ||
const errorMessage: string = error.message; | ||
throw new Error(errorMessage); | ||
}), | ||
); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there is no login token we should be logging out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming its handled in getJWTLoginToken()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not being handled in getJWTLoginToken()
However we do have a logout function, maybe we can create new PR for it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kshitij-k-osmosys Okay can do in New PR.