feat(cli): expand Slack imports across conversation types - #1
Conversation
Merge separate Slackdump roots, preserve private conversation visibility, adopt existing channel shells, open native DMs safely, render richer message content, and expand resumable dry-run accounting. Co-authored-by: nicknack5050 <nick@lucid.rocks> Signed-off-by: nicknack5050 <nick@lucid.rocks>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Thank you so much @nicknack5050 Two things I'd want fixed before merging. The DM collision guard doesn't cover every case. The relay keys The fix wants to live somewhere selection can't reach. In if let Some((other, _)) = self.state.channels.iter()
.find(|(id, s)| id.as_str() != channel.id && s.uuid == uuid.to_string())
{
return Err(CliError::Usage(format!(
"Slack {} {} resolves to the Buzz DM already imported for {other}; \
refusing to merge separate histories",
channel.kind.as_str(), channel.id
)));
}
Two I'd like fixed but wouldn't hold the PR over: Adopted channels can be archived in Buzz while Slack thinks they're active. The unarchive at
Smaller stuff, take or leave: The In the CSV parser (
If a run errors out mid-channel, an adopted archived channel stays unarchived until the next run puts it back. Correct behaviour, just worth a line in Get the DM guard and the kind constant sorted and I'll take it. The other two can be a follow-up. |
Co-authored-by: nicknack5050 <nick@lucid.rocks> Signed-off-by: nicknack5050 <nick@lucid.rocks>
|
Addressed the review feedback in
I left the optional file-splitting suggestion unchanged in this correctness pass so the review fix does not add a broad structural refactor; it can be a focused follow-up. Verified at the exact commit above:
|
|
Addressed the review feedback in 6385727.\n\n- Added a ledger-wide native-DM UUID collision guard after relay resolution (and on resume), with a regression for D1 already recorded and D2 resolving to the same Buzz DM.\n- Switched DM-open construction to while retaining import provenance tags.\n- First-time adopted channels now attempt unarchive regardless of Slack's archived flag; only the relay's exact “channel is not archived” result is accepted as a no-op.\n- Made optional per split export root while preserving the overall no-conversations rejection.\n- Corrected duplicate Slack-ID diagnostics before Buzz target-UUID reuse checks.\n- Hardened CSV parsing for whitespace before quoted fields and bare-CR records.\n- Escaped Markdown metacharacters in rendered author prefixes.\n- Documented the interrupted adopted-channel reopen/restore behavior.\n\nI left the optional file-splitting suggestion unchanged in this correctness pass so the review fix does not add a broad structural refactor; it can be a focused follow-up.\n\nVerified at the exact commit above:\n\n- test result: ok. 296 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 2.02s running 0 tests test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s running 1 test test result: ok. 0 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 0.00s: 296 passed\n- : passed\n- : passed |
|
Re-reviewed Two things came out of the fixes. The author escape breaks the desktop prefix strip. This one's on me for not checking the consumer when I suggested it.
const prefix = `**${importAuthor.displayName}**: `;
return body.startsWith(prefix) ? body.slice(prefix.length) : body;
The prefix ends up rendering as visible duplicate text on every message that person wrote, which is worse than the mangled bold it was fixing. Mirroring the escape in
The problem is that the error never leaves the relay. The part actually worth fixing is the other half of that comment — "every other failure must stop the import before writes" doesn't hold either. If the unarchive fails for a real reason on a genuinely archived channel you still get Since a 9002 ack tells you nothing, the only way to know is to look: read the channel back after the unarchive and bail if it's still archived. Or leave the behaviour and drop the claim from the comment, because a comment describing a guarantee the relay doesn't make is worse than no comment. I don't think this blocks either way. One thing I checked because it would have been genuinely bad: the relay matches metadata tags individually ( Smaller notes: Fix the desktop prefix and this is good to go from my side. |
Co-authored-by: nicknack5050 <nick@lucid.rocks> Signed-off-by: nicknack5050 <nick@lucid.rocks>
|
Fixed every known review follow-up in
Exact-commit validation:
The remote PR head matches the verified commit. No Slack or Buzz history was written during validation. |
…3813) ## What Clearing an edit to empty and hitting accept now **deletes the message** instead of hanging. One of Sam's frequent workflows is to delete a message by editing it, clearing the text, and pressing Enter — which previously no-op'd (a deliberate guard blocked empty edits). ## How Pure client-side wiring — **no relay, schema, or Rust changes.** 1. **`MessageComposer.tsx`** — the edit path had a guard that *blocked* empty edits (`if (!trimmed && !hasMedia) return;`). That guard is simply **removed**, so empty content flows through the normal edit path to `onEditSave("", [], [])`. `buildOutgoingMessage("")` is a safe no-op. 2. **`handleEditSave` in `useChannelPaneHandlers.ts`** — when an edit is submitted with empty text and no media tags, it exits edit mode and opens the **same "Delete message?" confirmation** the Delete menu action shows, rather than publishing an empty edit. 3. **`DeleteMessageConfirmDialog.tsx`** — the confirmation dialog, extracted into **one shared component**. `MessageActionBar` renders it for the Delete menu action (previously inline), and `ChannelScreen` renders it for the empty-edit path. No duplicated dialog UI. **Delete** runs the existing `deleteMutate`; **Cancel** leaves the message untouched. Because both the main timeline and the thread panel already route edit-save through `handleEditSave`, this covers both surfaces with a single dialog at the `ChannelScreen` level — no per-composer plumbing. - Image-only edits (empty text but attachments present) still publish normally — only a *fully* empty edit prompts to delete. - An empty edit can never publish an empty body: `handleEditSave` returns before the edit mutation. ## Review history This PR was reworked three times in response to review — each pass made it smaller: 1. First cut wrapped this in a new "Delete message?" `AlertDialog` rendered from a composer hook — a verbatim duplicate of the confirmation already in `MessageActionBar.tsx`. Removed. 2. Second cut threaded a dedicated `onDeleteEditTarget` callback down `ChannelScreen → ChannelPane → MessageComposer / MessageThreadPanel`. Also redundant — the delete decision moved entirely into `handleEditSave`, which every edit-save already flows through. 3. Third cut added a special-case empty branch to the composer, which pushed `MessageComposer.tsx` over the file-size ratchet and led to an unrelated emoji-helper extraction to make room. Both gone: deleting the pre-existing guard (rather than adding a branch) is net-negative, so there's no ratchet pressure and **nothing emoji-related in this PR**. `MessageComposer.types.ts` is back to baseline too. 4. Fourth pass (this one): an unconfirmed, no-undo delete was too sharp. The empty-edit path now routes through the same **"Delete message?" confirmation** as the menu action — shared as one `DeleteMessageConfirmDialog` component (so it's reuse, not the duplicate dialog from cut #1). ## Testing - **E2E:** `desktop/tests/e2e/empty-edit-delete.spec.ts` (Playwright, smoke project), three tests, all passing locally: - *clearing an edit to empty prompts to delete, then deletes on confirm* — edits the mock identity's own `#general` message, clears it, Enter → the **"Delete message?"** dialog appears; Delete → the row disappears and edit mode exits. - *cancelling the empty-edit delete keeps the message* — same up to the dialog, then Cancel → the message survives. - *a non-empty edit still edits and never deletes* — guards the other direction (no dialog). - `pnpm typecheck`, biome, file-size + px-text guards all clean; full desktop unit suite (3847 tests) passing locally. > Heads-up for the reviewer: pushed with `--no-verify` because the pre-push hook runs the Rust **integration** suite, which needs Docker (Postgres/Redis) that isn't available in this environment — it doesn't apply to this desktop-only change. CI runs the real gates. --- 🐝 Built by Bumble in Buzz, from a conversation in #test-swesterman. --------- Signed-off-by: Sam Westerman <swesterman@squareup.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 participants
Summary
This is a focused extension of
block/buzz#2704, built directly on its current head (d3a4a949). Merging this stacked PR intoRenKoya1:feat/slack-importwill update the existing Block/Buzz PR without duplicating its full history againstmain.--export-dirroots and merge Slack/Slackdumpchannels.json,groups.json,dms.json,mpims.json,users.json, andorg_users.jsonSafety properties
Validation
Exact commit:
42f2c33add4450bc479ba910b419180da909ac4bcargo fmt --all -- --checkcargo test -p buzz-cli— 290 passed, 0 failedcargo clippy -p buzz-cli --all-targets -- -D warningsgit diff --cached --checkThe Lucid audit was dry-run only: it did not connect to the relay, write an import state file, or import Slack history.
Remaining limits
Slack-hosted files are linked but not re-hosted; same-emoji reactions remain one bot-signed reaction rather than preserving every reactor identity; edit/delete history is not reconstructed; and interactive Slack app actions, workflows, and custom emoji are not replayed.
Originating Buzz thread:
buzz://message?channel=d9eedfbf-4b00-4ab2-a574-5859cd2c3d84&id=74baa551b197701ed487a0538796135998585d709cb2b0055b832f903cd54075