Skip to content
This repository has been archived by the owner on Apr 12, 2022. It is now read-only.

Remove Manual Groups #2681

Merged
merged 9 commits into from
Dec 21, 2021
Merged

Remove Manual Groups #2681

merged 9 commits into from
Dec 21, 2021

Conversation

evantahler
Copy link
Member

@evantahler evantahler commented Dec 14, 2021

To simplify & clarify Grouparoo, this PR removes the concept of "manual" Groups. From this point forward, all Grouparoo Groups are "calculated", meaning that they dynamically determine which Records are members based on the Properties of those Records. This has the effect of simplifying many things from the UI, configuration files, and internal logic. This is a breaking change.

After the application of this PR, the following breaking changes will happen:

  • The migration will fail if there are manual Groups in the database. These will need to be removed before the migration can be applied
  • If there are manual Groups in code config, Grouparoo will fail to validate/apply/start/run with an error message
  • If there are calculated Groups in code config, Grouparoo will boot, and provide a deprecation warning that group.type is no longer a property of a Grouparoo group.

Docs @ grouparoo/www.grouparoo.com#828

Checklists

Development

  • Application changes have been tested appropriately

Impact

  • Code follows company security practices and guidelines
  • Security impact of change has been considered
  • Performance impact of change has been considered
  • Possible migration needs considered (model migrations, config migrations, etc.)

Please explain any security, performance, migration, or other impacts if relevant:

Code review

  • Pull request has a descriptive title and context useful to a reviewer. Screenshots or screencasts are attached where applicable.
  • Relevant tags have been added to the PR (bug, enhancement, internal, etc.)

@evantahler evantahler added enhancement New feature or request breaking change this release may have ramifications for your cluster or data labels Dec 14, 2021
@evantahler evantahler changed the title WIP - Removing Manual Groups Remove Manual Groups Dec 16, 2021
Comment on lines +46 to +56
@Column(DataType.STRING)
op: GroupRuleOpType;

@Column
relativeMatchNumber: number;

@Column
relativeMatchUnit: string;
@Column(DataType.STRING)
relativeMatchUnit: RelativeMatchUnitType;

@Column
relativeMatchDirection: string;
@Column(DataType.STRING)
relativeMatchDirection: RelativeMatchDirectionType;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adds better TS Types

Comment on lines +58 to +62
operation: { op: GroupRuleOpType; description?: string };
match?: string | number | boolean;
relativeMatchNumber?: number;
relativeMatchUnit?: string;
relativeMatchDirection?: string;
relativeMatchUnit?: RelativeMatchUnitType;
relativeMatchDirection?: RelativeMatchDirectionType;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adds better TS Types


export async function loadGroup(
configObject: GroupConfigurationObject,
externallyValidate: boolean,
validate = false
): Promise<IdsByClass> {
let isNew = false;
validateConfigObjectKeys(Group, configObject, ["rules"]);
validateConfigObjectKeys(Group, configObject, ["rules", "type"]);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, we won't crash if group.type is provided on a config object, but we check it below

Comment on lines +103 to +110
export type GroupRuleOpType =
| typeof _boolean_ops[number]["op"]
| typeof _number_ops[number]["op"]
| typeof _string_ops_sqlite[number]["op"]
| typeof _string_ops_postgres[number]["op"]
| typeof _date_ops[number]["op"];
export type RelativeMatchUnitType = typeof _relativeMatchUnits[number];
export type RelativeMatchDirectionType = "add" | "subtract";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally providing types for the Group Rules

@evantahler evantahler marked this pull request as ready for review December 16, 2021 05:21
Copy link
Member

@pedroslopez pedroslopez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a big one! Looks good though and I think the migration path is clear.

Copy link
Contributor

@teallarson teallarson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bye Manual Groups! I found one spot that still used "Calculated Group" instead of "Group" in a test, but other than that looks good.

core/__tests__/integration/happyPath.ts Outdated Show resolved Hide resolved
@evantahler evantahler merged commit 65aea87 into main Dec 21, 2021
@evantahler evantahler deleted the 179691483-remove-manual-groups branch December 21, 2021 19:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking change this release may have ramifications for your cluster or data enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants