Allow a notification keyword to start with a dot (#34574)
Both notification settings tabs stored a keyword under a push rule whose id was the keyword itself. Homeservers reserve the rule ids beginning with a dot for the rules they define, and Synapse refuses to create any other, so saving a keyword such as "...push complete" failed outright and the whole save was reported as an error. The rule id is an internal name that the user never sees — both tabs list keywords by the rule's pattern — so it is the id that gives way. It now drops the leading dots while the pattern keeps the keyword exactly as it was typed, which is what is matched against messages. Two keywords can want the same id that way, so a number is appended when one is taken, which also keeps "banana" from overwriting the rule for ".banana". Tests: the shared reconciler and the older tab both store a dotted keyword under an accepted id, and two keywords differing only by a leading dot get an id each.
This commit is contained in:
@@ -53,6 +53,7 @@ import { SettingsSubsection } from "./shared/SettingsSubsection";
|
||||
import { doesRoomHaveUnreadMessages } from "../../../Unread";
|
||||
import SettingsFlag from "../elements/SettingsFlag";
|
||||
import { onSubmitPreventDefault } from "../../../utils/form.ts";
|
||||
import { keywordRuleId } from "../../../models/notificationsettings/keywordRuleId.ts";
|
||||
|
||||
// TODO: this "view" component still has far too much application logic in it,
|
||||
// which should be factored out to other files.
|
||||
@@ -565,13 +566,16 @@ export default class Notifications extends React.PureComponent<EmptyObject, ISta
|
||||
ruleVectorState = existingRuleVectorState ?? VectorState.On; //default
|
||||
}
|
||||
const kind = PushRuleKind.ContentSpecific;
|
||||
const ruleIds = new Set(originalRules.map((r) => r.rule_id));
|
||||
for (const word of diff.added) {
|
||||
await MatrixClientPeg.safeGet().addPushRule("global", kind, word, {
|
||||
const ruleId = keywordRuleId(word, ruleIds);
|
||||
ruleIds.add(ruleId);
|
||||
await MatrixClientPeg.safeGet().addPushRule("global", kind, ruleId, {
|
||||
actions: PushRuleVectorState.actionsFor(ruleVectorState),
|
||||
pattern: word,
|
||||
});
|
||||
if (ruleVectorState === VectorState.Off) {
|
||||
await MatrixClientPeg.safeGet().setPushRuleEnabled("global", kind, word, false);
|
||||
await MatrixClientPeg.safeGet().setPushRuleEnabled("global", kind, ruleId, false);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,31 @@
|
||||
/*
|
||||
Copyright 2026 hayaksi1
|
||||
|
||||
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
|
||||
Please see LICENSE files in the repository root for full details.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Work out which rule ID a keyword should be stored under.
|
||||
*
|
||||
* A keyword is normally its own rule ID, but homeservers reserve the IDs beginning with a dot for
|
||||
* the rules they define themselves and refuse to store any other, so a keyword such as
|
||||
* `...push complete` cannot be one. The ID drops those leading dots; the rule's pattern, which is
|
||||
* what messages are actually matched against, keeps the keyword exactly as it was typed. That can
|
||||
* leave two keywords wanting the same ID, so one of them is numbered off the end.
|
||||
*
|
||||
* @param keyword - The keyword as the user typed it.
|
||||
* @param existingIds - The rule IDs already in use.
|
||||
* @returns A rule ID the homeserver will accept and no other rule is using.
|
||||
*/
|
||||
export function keywordRuleId(keyword: string, existingIds: Iterable<string>): string {
|
||||
// A keyword of nothing but dots leaves nothing to name the rule after.
|
||||
const base = keyword.replace(/^\.+/, "") || "keyword";
|
||||
|
||||
const taken = new Set(existingIds);
|
||||
if (!taken.has(base)) return base;
|
||||
|
||||
let suffix = 2;
|
||||
while (taken.has(`${base}-${suffix}`)) suffix++;
|
||||
return `${base}-${suffix}`;
|
||||
}
|
||||
@@ -15,6 +15,7 @@ import { RoomNotifState } from "../../RoomNotifs";
|
||||
import { type NotificationSettings } from "./NotificationSettings";
|
||||
import { type PushRuleDiff, type PushRuleUpdate } from "./PushRuleDiff";
|
||||
import { buildPushRuleMap } from "./PushRuleMap";
|
||||
import { keywordRuleId } from "./keywordRuleId";
|
||||
|
||||
function toStandardRules(
|
||||
model: NotificationSettings,
|
||||
@@ -225,9 +226,12 @@ export function reconcileNotificationSettings(
|
||||
}
|
||||
newKeywords.delete(rule.pattern!);
|
||||
}
|
||||
const ruleIds = new Set(contentRules.map((rule) => rule.rule_id));
|
||||
for (const keyword of newKeywords) {
|
||||
const ruleId = keywordRuleId(keyword, ruleIds);
|
||||
ruleIds.add(ruleId);
|
||||
changes.added.push({
|
||||
rule_id: keyword,
|
||||
rule_id: ruleId,
|
||||
kind: PushRuleKind.ContentSpecific,
|
||||
default: false,
|
||||
enabled: model.mentions.keywords,
|
||||
|
||||
@@ -858,6 +858,20 @@ describe("<Notifications />", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("adds a keyword that starts with a dot", async () => {
|
||||
await getComponentAndWait();
|
||||
|
||||
await userEvent.type(screen.getByLabelText("Keyword"), ".jest");
|
||||
|
||||
fireEvent.click(screen.getByText("Add"));
|
||||
|
||||
// The homeserver rejects a rule id beginning with a dot, so only the pattern keeps it.
|
||||
expect(mockClient.addPushRule).toHaveBeenCalledWith("global", PushRuleKind.ContentSpecific, "jest", {
|
||||
actions: [PushRuleActionName.Notify, { set_tweak: "highlight", value: false }],
|
||||
pattern: ".jest",
|
||||
});
|
||||
});
|
||||
|
||||
it("adds a new keyword with same actions as existing rules when keywords rule is off", async () => {
|
||||
const offContentRule = {
|
||||
...bananaRule,
|
||||
|
||||
@@ -215,4 +215,29 @@ describe("NotificationSettings", () => {
|
||||
expect(pendingChanges.deleted).toHaveLength(0);
|
||||
expect(pendingChanges.updated).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("stores a keyword that starts with a dot under an id the server will accept", async () => {
|
||||
const pushRules = (await import("./pushrules_default.json")) as IPushRules;
|
||||
const model = { ...DefaultNotificationSettings, keywords: ["...push complete"] };
|
||||
|
||||
const pendingChanges = reconcileNotificationSettings(pushRules, model, false);
|
||||
|
||||
expect(pendingChanges.added).toEqual([
|
||||
expect.objectContaining({
|
||||
kind: PushRuleKind.ContentSpecific,
|
||||
rule_id: "push complete",
|
||||
pattern: "...push complete",
|
||||
}),
|
||||
]);
|
||||
});
|
||||
|
||||
it("keeps the ids of two keywords that differ only by a leading dot apart", async () => {
|
||||
const pushRules = (await import("./pushrules_default.json")) as IPushRules;
|
||||
const model = { ...DefaultNotificationSettings, keywords: ["banana", ".banana"] };
|
||||
|
||||
const pendingChanges = reconcileNotificationSettings(pushRules, model, false);
|
||||
|
||||
expect(pendingChanges.added.map((rule) => rule.rule_id)).toEqual(["banana", "banana-2"]);
|
||||
expect(pendingChanges.added.map((rule) => rule.pattern)).toEqual(["banana", ".banana"]);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user