Skip to content

directory: only create group links after approval - #7356

Open
epoberezkin wants to merge 10 commits into
stablefrom
ep/directory-link
Open

directory: only create group links after approval#7356
epoberezkin wants to merge 10 commits into
stablefrom
ep/directory-link

Conversation

@epoberezkin

Copy link
Copy Markdown
Member

No description provided.

@epoberezkin
epoberezkin changed the base branch from master to stable August 8, 2026 21:47
simplex-chat-agent[bot]
simplex-chat-agent Bot previously approved these changes Aug 8, 2026

@simplex-chat-agent simplex-chat-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Summary

This branch moves group-link creation in the directory service from group-join time to first admin approval. The join no longer calls APICreateGroupLink; instead the service goes straight to GRSPendingApproval and creates (or, for legacy/re-registrations, re-syncs) the link inside /approve (approvedGroupLink). The old GroupProfileUpdate/welcome-message-link machinery is replaced by a single linkOnlyChange check that keeps an active group listed when only the bot link or whitespace changes, and a new updateGroupLinksFromApp config flag disables core's automatic link-data refresh so the service issues every update itself. Search/cards can now resolve a submitted link to its registered group. Docs, README, plan, and tests are updated to match.

I read Service.hs in full, plus the Listing.hs, Util.hs, Subscriber.hs, Controller.hs and Chat.hs changes, the plan, docs, and the event definition, and traced the new state transitions and the approval/link-creation paths. The implementation matches the plan and the transition table.

Verified points worth noting:

  • APICreateGroupLink embeds GroupShortLinkData {groupProfile, publicGroupData = Nothing} at creation, so the fresh link in the not-found branch carries the profile even though it (correctly) skips the full setGroupLinkData relay/publishable-data sync — that path is only taken for non-public groups where relays don't apply. The found/re-approval branch does call updateGroupLinkData. Consistent with §2.2.
  • linkOnlyChange short-circuits on dn == dn' before evaluating descrWords, so the old-name label used in stripping is safe.
  • No new code sets GRSPendingUpdate; it is handled only as a legacy status. The removed GroupProfileUpdate, unexpectedError, and find import leave no dangling references.
  • Non-admin link lookups return "no groups found" for non-active registrations, so registered-but-unlisted groups are not revealed — good privacy behavior.

No blocking issues found. One maintainability note (not blocking): the recommended link line "Link to join the group " <> name <> ": " is now duplicated across four sites — the approval message (Service.hs:1274), the search-result line (Service.hs:1464), the listing line (Listing.hs:116), and, critically, the label that linkOnlyChange strips to detect link-only edits (Service.hs:585). The "adding the recommended line keeps the group listed" behavior depends on the stripping phrase matching the recommended phrase. It works today (and withoutLink removes the actual link token independently, so a whitespace mismatch is harmless), but a future wording change in one place could silently change re-approval behavior. Worth a shared constant if this text is touched again.

Approving.

simplex-chat-agent[bot]
simplex-chat-agent Bot previously approved these changes Aug 9, 2026

@simplex-chat-agent simplex-chat-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The branch moves group-link creation from group-join to admin approval, and makes adding the link to the welcome message optional (recommended, not required). The state machine collapses PendingUpdate into PendingApproval, and the GroupProfileUpdate classification (GPServiceLink*) is replaced by a simpler "is this only a link/whitespace change?" test. This is a coherent, well-executed change; docs (README, DIRECTORY.md) are updated to match.

Things I checked that are correct:

  • The new LEFT JOIN user_contact_links in groupReqQuery cannot duplicate group rows. user_contact_links has a unique index on group_id alone (idx_user_contact_links_group_id), so at most one link row per group. This replaces the previous per-group getGroupLink (N+1) with a single join — a genuine performance win, and searchListedGroups' separate count query stays consistent.
  • toMaybeGroupLink is consistent with the canonical toGroupLink. The Maybe BoolInt widening on short_link_data_set/short_link_large_data_set only matters for the all-NULL (no link) case, which correctly yields Nothing.
  • updateGroupLinksFromApp is scoped correctly. It defaults to False (mobile/CLI keep the automatic background link refresh) and is only set True by the directory service, which now refreshes link data explicitly (updateGroupLinkData at approval and on link-only changes to active groups). No regression for the app.
  • linkOnlyChange symmetrically strips the recommended link line (link token via matchesGroupLink plus the groupLinkLinePrefix text) from both old and new descriptions, so adding/removing the recommended line keeps a group Active while any other edit triggers re-approval — matching the documented behavior.
  • Approval correctly fails closed: if the service can't create the link (not admin / removed), approvedGroupLink returns Left and the group stays PendingApproval rather than being listed without a working link.

Minor:

  • One user-facing typo on a modified line ("upgated" → "updated"), suggested inline.
  • Cosmetic: the /link command's menu button description in directoryCommands still reads "View and upgrade group link" (Service.hs:242), while the /help text was changed to "view group link". Not in the diff, so no inline suggestion — worth aligning.

No correctness, concurrency, or security issues found.

Comment thread apps/simplex-directory-service/src/Directory/Service.hs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants