Merged
Conversation
Signed-off-by: sanjaykhondal-aw <sanjay.khondal@ayanworks.com>
Signed-off-by: sanjaykhondal-aw <sanjay.khondal@ayanworks.com>
Signed-off-by: sanjaykhondal-aw <sanjay.khondal@ayanworks.com>
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to aa4bb3f in 1 minute and 59 seconds. Click for details.
- Reviewed
702lines of code in12files - Skipped
0files when reviewing. - Skipped posting
7draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. nextjs/src/config/CommonConstant.ts:75
- Draft comment:
Verify that setting the predicate condition value to 'Select' (instead of an empty string) meets the intended placeholder behavior. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify their intention regarding a specific change. This violates the rule against asking the author to confirm their intention or ensure behavior is intended. The comment does not provide a specific suggestion or ask for a test to be written, which would have been acceptable.
2. nextjs/src/features/verification/components/ConnectionListCells.tsx:54
- Draft comment:
The custom checkbox cell uses both a button onClick and a hidden input onChange. Ensure these handlers don’t trigger duplicate events. Consider consolidating the event handling. - Reason this comment was not posted:
Comment looked like it was already resolved.
3. nextjs/src/features/wallet/DidListComponent.tsx:142
- Draft comment:
Avoid using window.location.reload() in SPA context; consider using router.refresh() to update the view instead. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. nextjs/src/features/wallet/DidListComponent.tsx:407
- Draft comment:
Using setPrivateKeyValue(privateKeyValue || privateKey) may prevent state update if privateKeyValue is already truthy. Consider using the new private key directly. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. nextjs/src/features/wallet/DidListComponent.tsx:295
- Draft comment:
The useEffect calling fetchOrganizationDetails has an empty dependency array. If orgId can change, consider including it in the dependency array. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. nextjs/src/components/RoleViewButton.tsx:33
- Draft comment:
Typo: 'Features.CRETAE_ORG' should likely be 'Features.CREATE_ORG'. - Reason this comment was not posted:
Marked as duplicate.
7. nextjs/src/features/profile/components/DevelopersSetting.tsx:25
- Draft comment:
Typo/lexicographical note: The property name 'organisation' (in the UserOrgRole interface) is spelled using British English, while the 'Organization' interface uses American English. Consider unifying the spelling to improve consistency. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% While the comment is technically correct about the inconsistency, we need to consider if this is actually worth fixing. The inconsistency might be intentional if one matches an API response format. Without knowing the external constraints, changing this could actually break functionality. Additionally, this is a relatively minor issue that doesn't affect functionality. I might be underestimating the value of consistent naming in a codebase. Inconsistent naming can lead to confusion and bugs over time. While consistency is valuable, this particular case seems more likely to be tied to external API constraints, and forcing consistency could cause problems. The risk of breaking changes outweighs the benefit. The comment should be removed. While it points out a real inconsistency, changing it could break API compatibility and the benefit doesn't justify the risk.
Workflow ID: wflow_ZmVyfOiwdCcKWD14
You can customize 
| const role = useAppSelector((state) => state.organization.orgInfo?.roles[0]) | ||
| const isRoleAccess = (): boolean => { | ||
| switch (feature) { | ||
| case Features.CRETAE_ORG: |
Contributor
There was a problem hiding this comment.
Possible typo: 'CRETAE_ORG' may be intended as 'CREATE_ORG'. Please verify the enum naming.
Suggested change
| case Features.CRETAE_ORG: | |
| case Features.CREATE_ORG: |
Signed-off-by: sanjaykhondal-aw <sanjay.khondal@ayanworks.com>
Contributor
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed b0e6364 in 1 minute and 37 seconds. Click for details.
- Reviewed
26lines of code in2files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. nextjs/src/common/enums.ts:38
- Draft comment:
Corrected enum name from CRETAE_ORG to CREATE_ORG ensures consistency. Verify similar constants (e.g., CRETAE_SCHEMA) if needed. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =50%<= threshold50%The comment acknowledges a correction in the enum name, which is a valid observation. However, it also asks the author to verify similar constants, which is against the rules as it asks for verification. The first part of the comment is useful, but the second part is not allowed.
2. nextjs/src/components/RoleViewButton.tsx:32
- Draft comment:
Updated switch case to use Features.CREATE_ORG maintains consistency with the enums. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment seems to be purely informative, as it is just stating what was done without providing any actionable feedback or suggestions. It doesn't ask for confirmation or suggest any changes.
3. nextjs/src/common/enums.ts:40
- Draft comment:
Typo: 'CRETAE_SCHEMA' appears to be misspelled. Consider renaming it to 'CREATE_SCHEMA' for consistency and clarity. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_k0JqFsWI3wfHjrGW
You can customize
Signed-off-by: sanjaykhondal-aw <sanjay.khondal@ayanworks.com>
Contributor
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed 936a3ee in 1 minute and 42 seconds. Click for details.
- Reviewed
20lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. nextjs/src/features/dashboard/components/SchemasList.tsx:255
- Draft comment:
Disabled buttons may not trigger onClick, so tooltip feedback (set in handleCreateSchemaClick) won’t display. Consider a wrapper or alternative event (e.g. onMouseEnter) for showing the tooltip when disabled. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment raises a valid technical point - disabled buttons often don't trigger click events. However, looking at the code structure, the tooltip is actually controlled by a separate TooltipTrigger wrapper component from a UI library, not directly by the onClick handler. The tooltip appears to be using a controlled 'open' prop (showTooltip state), which suggests it works independently of button clicks. I could be wrong about how the TooltipTrigger component works internally. Maybe it does rely on button events propagating. Even if I'm uncertain about the internal tooltip implementation, the code shows the tooltip state is controlled externally via the 'open' prop, which suggests it should work regardless of button state. The comment should be deleted as it appears to misunderstand how the tooltip is implemented in this case - it's controlled via state rather than click events.
Workflow ID: wflow_P9HpLIo84cKJ9l6U
You can customize
| <Button onClick={handleCreateSchemaClick}> | ||
| <Button | ||
| onClick={handleCreateSchemaClick} | ||
| disabled={ |
Contributor
There was a problem hiding this comment.
Consider extracting the permission check (orgId, walletExists, role check) into a shared variable to avoid duplicating logic in both the disabled prop and handleCreateSchemaClick.
Signed-off-by: sanjaykhondal-aw <sanjay.khondal@ayanworks.com>
shitrerohit
approved these changes
Jun 12, 2025
Contributor
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 6c70748 in 37 seconds. Click for details.
- Reviewed
12lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. nextjs/src/features/schemas/components/SchemaList.tsx:86
- Draft comment:
Removed outdated commented state initialization. Using the properly typed useState<ISchemaData[]>([]) improves code clarity. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_GkDSiHXSRWZLhRMi
You can customize
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WHAT
Important
Fixes role-based access, UI inconsistencies, and TypeScript errors across multiple components.
RoleViewButton.tsxby simplifying role checks for features likeCREATE_ORG,ISSUANCE, andVERIFICATION.app-sidebar.tsx.ConnectionListCells.tsxfor the connection list table.CommonConstant.ts.SchemasList.tsxandDevelopersSetting.tsx.enums.tsby renamingCRETAE_ORGtoCREATE_ORG.disabledprop toEmptyMessage.tsxfor better button control.This description was created by
for 6c70748. You can customize this summary. It will automatically update as commits are pushed.