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
Update Event Source & Channel Sources for Add flow #6294
Update Event Source & Channel Sources for Add flow #6294
Conversation
6dc29d7
to
e2f0860
Compare
// eslint-disable-next-line react-hooks/exhaustive-deps | ||
}, []); |
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.
I think it's okay to keep the dependency here
// eslint-disable-next-line react-hooks/exhaustive-deps | |
}, []); | |
}, [setModelRefs]); |
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.
Updated.
@@ -66,8 +66,7 @@ export const useChannelList = (namespace: string): ChannelListProps => { | |||
}) | |||
// eslint-disable-next-line no-console | |||
.catch((err) => console.warn(err.message)); |
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.
Can you please add a console warning msg here as well
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.
Added
b5573b3
to
645e9df
Compare
React.useEffect(() => { | ||
_.forIn(channelResourcesList, (channelRef: string) => { | ||
const accessList = []; | ||
_.forIn(channelResourcesList.channels, (channelRef: string) => { |
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.
can we add the .loaded
condition here and wrap for both forIn and promise in it.
@@ -36,10 +36,10 @@ export const getChannelKind = (ref: string): string => { | |||
|
|||
export const useChannelList = (namespace: string): ChannelListProps => { | |||
const [accessData, setAccessData] = useSafetyFirst({ loaded: false, channelList: [] }); | |||
const channelResourcesList = getDynamicChannelModelRefs(); | |||
const accessList = []; | |||
const channelResourcesList = useChannelResourcesList(); |
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.
better to destructure loaded and channels here
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.
Updated.
645e9df
to
cb5e86d
Compare
Verified locally works fine |
/approve Verified the changes , it works as expected i.e on installation of serverless and creating |
/retest |
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.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: invincibleJai, karthikjeeyar, rottencandy, sahil143 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
1 similar comment
/retest |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest |
/retest Please review the full test history for this PR and help us cut down flakes. |
4 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest |
/retest Please review the full test history for this PR and help us cut down flakes. |
3 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
Fixes:
https://issues.redhat.com/browse/ODC-4460
Analysis / Root cause:
Event Sources list gets fetched only on plugin load, and do not get updated after operator install.
Solution Description:
Use hooks to fetch and update Event & Channel sources when required in the Add page.
Unit test coverage report:
Unchanged.
Browser conformance: