Fix: tagging block due to unauthorise request on /user/:id

[DarrellRoberts]

Description

Tagging feature doesn't work because Coordinators can't fetch from /user/:id.

Now that personId is available on /user, we no longer need to fetch from /user/:id and can use the former endpoint

Related Issues

Closes #659

Changes

• Bullet list of meaningful changes (optional)

Screenshots / Demos

Checklist

• WITHIN THE SCOPE OF AN ISSUE; No unnecessary files included
• Tests added/updated
• Documentation updated
• CI passes

[nadavosa]

Code Review

3 findings — 2 confirmed bugs, 1 plausible unsafe cast.

---

1. CONFIRMED — Editing a tagged comment with a cold cache silently strips all tag notifications

src/components/Dashboard/Profile/sections/Comments/common/hooks/useCommentTag.tsx

initTags bails out early when users has not loaded yet:

const initTags = (value: string) => {
  if (!value || !users) return;  // returns undefined
  ...
};

handleSaveEdit in EntityComments.tsx calls initTags and then immediately builds taggedPersonIds from its result:

const currentTags = initTags(edit.editText);  // undefined if cache cold
currentTags?.forEach(...)                     // no-op
// taggedPersonIds stays []
updateComment({ text: formattedText.trim(), taggedPersonIds }, ...);

On first page load (TanStack Query cache cold), any coordinator who opens a comment to edit and saves — even without changing anything — will silently save with taggedPersonIds: [], stripping all existing tag notifications. There is no save button guard tied to whether users has loaded.

Fix: disable the Save button while the coordinators query is still loading, or let handleSaveEdit await the query before proceeding.

---

2. CONFIRMED — Re-saving a comment with a deactivated user's tag permanently corrupts the stored text

src/components/Dashboard/Profile/sections/Comments/common/hooks/useCommentTag.tsx

convertDbTextToEditable degrades gracefully to the placeholder @user when a tagged user is absent from the list. But initTags has no matching fallback — it silently skips users it cannot find:

matches.forEach((match) => {
  const user = users.find(...);
  if (user) {
    freshlyFoundTags.push({ ... });
  }
  // absent user → silently dropped, nothing pushed
});

handleSaveEdit re-encodes only what initTags returned, so the @user placeholder is saved verbatim:

Step	Before	After
DB text	Hello <@42>	—
Editable text (displayed)	Hello @user	—
After save	—	Hello @user (literal)

The original <@42> encoding is permanently lost, the tag notification is never sent, and any future render shows @user rather than the coordinator's name.

Fix: add an else branch in initTags that preserves the original <@id> syntax for users not found in the current list, rather than silently dropping them.

---

3. PLAUSIBLE — as number casts on nullable personId suppress the type warning but not the runtime null

src/components/Dashboard/Profile/sections/Comments/common/Autocomplete.tsx (two call sites) and src/components/Dashboard/Profile/sections/Comments/common/hooks/useCommentTag.tsx

ApiUserGet.personId is typed as number | null | undefined (via VoidableProps). The PR casts it with as number at three places to silence TypeScript, but null/undefined values pass through at runtime. For any coordinator account that is not yet linked to a Person record, personId will be null or undefined, which then gets stored in tag state and ends up in the taggedPersonIds array sent to the API.

Fix: add a null guard before storing the personId, e.g.:

if (personId == null) return; // or skip adding the tag

Or better: add personId as a required (non-nullable) field to the SDK type if the backend guarantees it, which removes the need for any cast.

[DarrellRoberts]

thanks @nadavosa ! I've pushed my latest changes which should address these edge cases

[nadavosa]

1. personId: -1 can reach the backend — in initTags, unresolvable users get personId: -1 pushed into freshlyFoundTags. On edit save, that passes the deduplication check and lands in taggedPersonIds sent to the backend. Either filter out personId: -1 before building taggedPersonIds, or don't push the fallback tag into freshlyFoundTags at all.

2. isUsersLoading disables comment button on page load — previously isTagFetch was only true during an explicit async action. Now the button is disabled while the initial users query runs on mount. Low friction but a behaviour change.

3. Truthy check on personId should be != null — if (user && user.personId) excludes personId === 0. Rest of the PR uses == null / != null consistently, same should apply here.

4. PR checklist is all unchecked and issue number is #ISSUE_ID — link the issue before merge.

[DarrellRoberts]

thanks @nadavosa , fixed