-
Notifications
You must be signed in to change notification settings - Fork 263
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
Refactor rules feature with new rules execution object contract + eslint license header rule update to support 2025 #7300
base: new-signup
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## new-signup #7300 +/- ##
===========================================
Coverage 32.02% 32.02%
===========================================
Files 41 41
Lines 893 893
Branches 204 220 +16
===========================================
Hits 286 286
+ Misses 607 557 -50
- Partials 0 50 +50
Flags with carried forward coverage won't be shown. Click here to find out more. |
@@ -0,0 +1,280 @@ | |||
/** | |||
* Copyright (c) 2024, WSO2 LLC. (https://www.wso2.com). |
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.
* Copyright (c) 2024, WSO2 LLC. (https://www.wso2.com). | |
* Copyright (c) 2025, WSO2 LLC. (https://www.wso2.com). |
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.
That's an extension rename of an existing file. Anyway let's teat it as a new file.
|
||
// TODO: Use this function to get the rule value. | ||
/* eslint-disable @typescript-eslint/no-unused-vars */ | ||
const handleGetRuleValue = () => { |
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.
const handleGetRuleValue = () => { | |
const handleGetRuleValue = (): void => { |
} = useGetRulesMeta("preIssueAccessToken"); | ||
|
||
// TODO: Use this function to get the rule value. | ||
/* eslint-disable @typescript-eslint/no-unused-vars */ | ||
const handleGetRuleValue = () => { |
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.
const handleGetRuleValue = () => { | |
const handleGetRuleValue = (): void => { |
</Button> | ||
</Divider> | ||
); | ||
|
||
return ( |
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.
We may need to add a placeholder for this component incase ruleConditions
are empty or in suspense.
if (!ruleConditions) {
return <Loading />;
}
return (
<>
// rest
</>
);
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.
Yes, need to add it.
|
||
/** | ||
* Method to get the context value | ||
* | ||
* @returns RuleInstanceData | ||
*/ | ||
export const getRuleInstanceValue = () => RuleContextRef.ruleInstance; | ||
export const getRuleInstanceValue = () => Object.freeze(RuleContextRef.ruleInstance); |
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.
We'll be moving to to a hook in the future right? Since the object is a nested object, Object.freeze
wouldn't be ideal.
Shall we do a deep clone, if we are planning to keep exposing the instance directly.
WDYT?
export const getRuleInstanceValue = () => Object.freeze(RuleContextRef.ruleInstance); | |
export const getRuleInstanceValue = (): RuleExecuteCollectionInterface => Object.freeze(RuleContextRef.ruleInstance); |
Purpose
$subject