feat(cc-widgets): added-address-book - #537
Conversation
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
cmullenx
left a comment
cmullenx
left a comment
There was a problem hiding this comment.
you will need to add props to show/hide consult/transfer to address book and show/hide transfer to entry point based on feature flags that you will receive from our app.
feat(cc-widgets): added-basic-playwright-for-address-book
adhmenon
added
the
run_e2e
| apiParams.search = search; | ||
| } | ||
|
|
||
| logger?.info(`CC-Components: Loading ${categoryName} - page: ${currentPage}, search: "${search}"`); |
There was a problem hiding this comment.
- Is logger optional? If not, we can remove the null check from here. Applicable everywhere
- We can add the context containing the file and method name in the same way we have at all the places. Applicable everywhere
There was a problem hiding this comment.
- In this case, we want logger to be optional as users need not specify a logger when they use our widgets or SDK...
- I will make that change here.
There was a problem hiding this comment.
- I thought this is our logging which gets uploaded to mats. If that's the case, we can't have this optional. Correct me if I am wrong
There was a problem hiding this comment.
I think the rationale behind this was since we use these components in our tests (both unit and e2e) - sometimes we don't provide the logger, hence it was made optional...
Again, I am alright changing it to compulsory but that not sure how it would affect tests.
Perhaps we can discuss this with Shreyas also.
| getEntryPoints = async (params?: EntryPointSearchParams): Promise<EntryPointListResponse> => { | ||
| try { | ||
| const response: EntryPointListResponse = await this.store.cc.getEntryPoints(params); | ||
| return response; | ||
| } catch (error) { | ||
| console.error('Error fetching entry points:', error); | ||
| return Promise.reject(error); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Do we need to create this method? This is just one line logic
There was a problem hiding this comment.
Yes, I feel so - we can catch any errors here in this method and it makes it uniform.
I feel we should retain this.
There was a problem hiding this comment.
console.error('Error fetching entry points:', error);Please use the logger here and also add proper logging at the top and for the success as well- We can simply throw the error from the catch block instead of calling the promise.reject
There was a problem hiding this comment.
Added here as well
| getAddressBookEntries = async (params?: AddressBookEntrySearchParams): Promise<AddressBookEntriesResponse> => { | ||
| try { | ||
| const response: AddressBookEntriesResponse = await this.store.cc.addressBook.getEntries(params ?? {}); | ||
| return response; | ||
| } catch (error) { | ||
| console.error('Error fetching address book entries:', error); | ||
| return Promise.reject(error); | ||
| } | ||
| }; |
There was a problem hiding this comment.
As mentioned above, I feel it makes sense to keep it as we cna do error handling and it is uniform.
There was a problem hiding this comment.
Same here. Let's improve the logging and error throwing
There was a problem hiding this comment.
Added here also.. not sure why it's not showing outdated.. code changes are there in the code tabs.
| {renderList( | ||
| queuesData.map((q) => ({id: q.id, name: q.name})), | ||
| (item) => handleQueueSelection(item.id, item.name, onQueueSelect, logger) | ||
| )} |
There was a problem hiding this comment.
Ideally we want to simplify this all we should see in presentational is
| {renderList( | |
| queuesData.map((q) => ({id: q.id, name: q.name})), | |
| (item) => handleQueueSelection(item.id, item.name, onQueueSelect, logger) | |
| )} | |
| {renderList(queuesData)} |
We can process the data in utils
There was a problem hiding this comment.
As mentioned above - this convolutes things and adds extra complexity to this PR... perhaps we can look at it later on.
# [1.28.0-ccwidgets.118](v1.28.0-ccwidgets.117...v1.28.0-ccwidgets.118) (2025-10-15) ### Features * **cc-widgets:** added-address-book ([#537](#537)) ([0997d10](0997d10))
|
🎉 This PR is included in version 1.28.0-ccwidgets.118 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
5 participants
COMPLETES #< CAI-7147 >
This pull request addresses
by making the following changes
Vidcast - https://app.vidcast.io/share/2f481e6b-227a-4652-92e9-0f228b48ca03
Change Type
The following scenarios were tested
The GAI Coding Policy And Copyright Annotation Best Practices
Checklist before merging