Skip to content

Commit

Permalink
Fix multiple groups bug
Browse files Browse the repository at this point in the history
  • Loading branch information
elie222 committed Jan 2, 2025
1 parent 980ca80 commit ff0b1f8
Show file tree
Hide file tree
Showing 3 changed files with 161 additions and 23 deletions.
153 changes: 144 additions & 9 deletions apps/web/utils/ai/choose-rule/match-rules.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,16 +74,18 @@ describe("findMatchingRule", () => {
});

it("matches a group rule", async () => {
const rule = getRule({ groupId: "group1" });

prisma.group.findMany.mockResolvedValue([
getGroup({
id: "group1",
items: [
getGroupItem({ type: GroupItemType.FROM, value: "[email protected]" }),
],
rule,
}),
]);

const rule = getRule({ groupId: "group1" });
const rules = [rule];
const message = getMessage({
headers: getHeaders({ from: "[email protected]" }),
Expand Down Expand Up @@ -135,23 +137,29 @@ describe("findMatchingRule", () => {
});

it("matches a rule with multiple conditions AND (category and group)", async () => {
const rule = getRule({
conditionalOperator: LogicalOperator.AND,
categoryFilters: [getCategory({ id: "category1" })],
groupId: "group1",
});

prisma.group.findMany.mockResolvedValue([
getGroup({
id: "group1",
items: [
getGroupItem({ type: GroupItemType.FROM, value: "[email protected]" }),
getGroupItem({
groupId: "group1",
type: GroupItemType.FROM,
value: "[email protected]",
}),
],
rule,
}),
]);
prisma.newsletter.findUnique.mockResolvedValue(
getNewsletter({ categoryId: "category1" }),
);

const rule = getRule({
conditionalOperator: LogicalOperator.AND,
categoryFilters: [getCategory({ id: "category1" })],
groupId: "group1",
});
const rules = [rule];
const message = getMessage({
headers: getHeaders({ from: "[email protected]" }),
Expand Down Expand Up @@ -269,6 +277,130 @@ describe("findMatchingRule", () => {
expect(result.rule?.id).toBe(rule.id);
expect(result.reason).toBe('Matched category: "category"');
});

it("should not match when item matches but is in wrong group", async () => {
const rule = getRule({
groupId: "correctGroup", // Rule specifically looks for correctGroup
});

// Set up two groups - one referenced by the rule, one not
prisma.group.findMany.mockResolvedValue([
getGroup({
id: "wrongGroup",
items: [
getGroupItem({
groupId: "wrongGroup",
type: GroupItemType.FROM,
value: "[email protected]",
}),
],
}),
getGroup({
id: "correctGroup",
items: [
getGroupItem({
groupId: "correctGroup",
type: GroupItemType.FROM,
value: "[email protected]",
}),
],
rule,
}),
]);

const rules = [rule];
const message = getMessage({
headers: getHeaders({ from: "[email protected]" }), // This matches item in wrongGroup
});
const user = getUser();

const result = await findMatchingRule(rules, message, user);

expect(result.rule).toBeUndefined();
expect(result.reason).toBeUndefined();
});

it("should match only when item is in the correct group", async () => {
const rule = getRule({ groupId: "correctGroup" });

// Set up two groups with similar items
prisma.group.findMany.mockResolvedValue([
getGroup({
id: "correctGroup",
items: [
getGroupItem({
groupId: "correctGroup",
type: GroupItemType.FROM,
value: "[email protected]",
}),
],
rule,
}),
getGroup({
id: "otherGroup",
items: [
getGroupItem({
groupId: "otherGroup",
type: GroupItemType.FROM,
value: "[email protected]", // Same value, different group
}),
],
}),
]);

const rules = [rule];
const message = getMessage({
headers: getHeaders({ from: "[email protected]" }),
});
const user = getUser();

const result = await findMatchingRule(rules, message, user);

expect(result.rule?.id).toBe(rule.id);
expect(result.reason).toContain("[email protected]");
});

it("should handle multiple rules with different group conditions correctly", async () => {
const rule1 = getRule({ id: "rule1", groupId: "group1" });
const rule2 = getRule({ id: "rule2", groupId: "group2" });

prisma.group.findMany.mockResolvedValue([
getGroup({
id: "group1",
items: [
getGroupItem({
groupId: "group1",
type: GroupItemType.FROM,
value: "[email protected]",
}),
],
rule: rule1,
}),
getGroup({
id: "group2",
items: [
getGroupItem({
groupId: "group2",
type: GroupItemType.FROM,
value: "[email protected]",
}),
],
rule: rule2,
}),
]);

const rules = [rule1, rule2];
const message = getMessage({
headers: getHeaders({ from: "[email protected]" }),
});
const user = getUser();

const result = await findMatchingRule(rules, message, user);

// Should match the first rule only
expect(result.rule?.id).toBe("rule1");
expect(result.reason).toContain("[email protected]");
});
});

function getRule(
Expand Down Expand Up @@ -329,8 +461,10 @@ function getCategory(overrides: Partial<Category> = {}): Category {
}

function getGroup(
overrides: Partial<Prisma.GroupGetPayload<{ include: { items: true } }>> = {},
): Prisma.GroupGetPayload<{ include: { items: true } }> {
overrides: Partial<
Prisma.GroupGetPayload<{ include: { items: true; rule: true } }>
> = {},
): Prisma.GroupGetPayload<{ include: { items: true; rule: true } }> {
return {
id: "group1",
name: "group",
Expand All @@ -339,6 +473,7 @@ function getGroup(
userId: "userId",
prompt: null,
items: [],
rule: null,
...overrides,
};
}
Expand Down
21 changes: 14 additions & 7 deletions apps/web/utils/ai/choose-rule/match-rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,18 +42,18 @@ async function findPotentialMatchingRules({
// groups singleton
let groups: Awaited<ReturnType<typeof getGroupsWithRules>>;
// only load once and only when needed
async function getGroups(rule: RuleWithActionsAndCategories) {
if (!groups) groups = await getGroupsWithRules(rule.userId);
async function getGroups(userId: string) {
if (!groups) groups = await getGroupsWithRules(userId);
return groups;
}

// sender singleton
let sender: { categoryId: string | null } | null | undefined;
async function getSender(rule: RuleWithActionsAndCategories) {
async function getSender(userId: string) {
if (typeof sender === "undefined") {
sender = await prisma.newsletter.findUnique({
where: {
email_userId: { email: message.headers.from, userId: rule.userId },
email_userId: { email: message.headers.from, userId },
},
select: { categoryId: true },
});
Expand Down Expand Up @@ -89,7 +89,8 @@ async function findPotentialMatchingRules({
// group
if (conditionTypes.GROUP) {
const { matchingItem } = await matchesGroupRule(
await getGroups(rule),
rule,
await getGroups(rule.userId),
message,
);
if (matchingItem) {
Expand All @@ -111,7 +112,10 @@ async function findPotentialMatchingRules({

// category
if (conditionTypes.CATEGORY) {
const match = await matchesCategoryRule(rule, await getSender(rule));
const match = await matchesCategoryRule(
rule,
await getSender(rule.userId),
);
if (match) {
unmatchedConditions.delete("CATEGORY");
if (typeof match !== "boolean")
Expand Down Expand Up @@ -189,10 +193,13 @@ export function matchesStaticRule(
}

async function matchesGroupRule(
rule: RuleWithActionsAndCategories,
groups: Awaited<ReturnType<typeof getGroupsWithRules>>,
message: ParsedMessage,
) {
return findMatchingGroup(message, groups);
const ruleGroup = groups.find((g) => g.rule?.id === rule.id);
if (!ruleGroup) return { group: null, matchingItem: null };
return findMatchingGroup(message, ruleGroup);
}

async function matchesCategoryRule(
Expand Down
10 changes: 3 additions & 7 deletions apps/web/utils/group/find-matching-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,10 @@ export async function getGroupsWithRules(userId: string) {

export function findMatchingGroup(
message: ParsedMessage,
groups: GroupsWithRules,
group: GroupsWithRules[number],
) {
for (const group of groups) {
const matchingItem = findMatchingGroupItem(message.headers, group.items);
if (matchingItem) {
return { group, matchingItem };
}
}
const matchingItem = findMatchingGroupItem(message.headers, group.items);
if (matchingItem) return { group, matchingItem };
return { group: null, matchingItem: null };
}

Expand Down

1 comment on commit ff0b1f8

@vercel
Copy link

@vercel vercel bot commented on ff0b1f8 Jan 2, 2025

Choose a reason for hiding this comment

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

Please sign in to comment.