126 lines
6.9 KiB
Markdown
126 lines
6.9 KiB
Markdown
# Batch 5–9 QA Report
|
||
|
||
## Build Results
|
||
|
||
| Check | Result | Notes |
|
||
|---|---|---|
|
||
| `cargo check --workspace` | ✅ PASS | All 10 crates compile cleanly |
|
||
| `cargo test --workspace` | ✅ PASS | 0 tests exist; 0 failures. **Note: test coverage is zero across the workspace** |
|
||
| `cargo clippy --workspace -- -D warnings` | ✅ PASS | Fixed multiple lint issues during this run |
|
||
| `cd frontend && npm run build` | ✅ PASS | Built in 2.67s. Chunk size warning (>500KB) is non-blocking |
|
||
|
||
### Lint Fixes Applied
|
||
- `cgcx-config`: `manual_range_contains` → used `!(MIN..=MAX).contains(&chunk)`
|
||
- `cgcx-db`: `explicit_auto_deref` → `&mut conn`; `too_many_arguments` on `ForwardRepo::insert`
|
||
- `cgcx-file-pipeline`: `collapsible_else_if` (2×), `clone_on_copy` on `Header`
|
||
- `cgcx-bot`: `useless_vec`, `redundant_closure` (auto-fixed), `collapsible_else_if`, `manual_strip`, `too_many_arguments` on `propagate_punishment`
|
||
- `cgcx-server`: `redundant_closure` (auto-fixed), `collapsible_else_if` (auto-fixed), `match` equality (auto-fixed)
|
||
|
||
---
|
||
|
||
## Regression Checklist
|
||
|
||
### 1. Review Buttons Work (approve, ignore, ban, blacklist, ban+blacklist)
|
||
| Item | Status | Evidence |
|
||
|---|---|---|
|
||
| `[ Approve ]` callback | ✅ | `main.rs:1910` — sets password hash, forwards media to destination, DMs user, edits review message to `[ APPROVED ]` |
|
||
| `[ Ignore ]` callback | ✅ | `main.rs:2067` — DMs user rejection, edits review message to `[ IGNORED ]` |
|
||
| `[ Blackl. ]` callback | ✅ | `main.rs:2079` — adds user to forward blacklist, edits review message to `[ BLACKLISTED ]` |
|
||
| `[ Ban ]` callback | ✅ | `main.rs:2087` — bans user in destination + review groups, inserts punishments, edits review message to `[ BANNED ]` |
|
||
| `[ Ban/BL u. ]` callback | ✅ | `main.rs:2102` — bans + blacklists, edits review message to `[ BAN/BL ]` |
|
||
| Admin permission gate | ✅ | `main.rs:1903` — checks `is_admin_in_chat` for review group before processing action |
|
||
|
||
**Risk:** None of these flows have unit/integration tests.
|
||
|
||
---
|
||
|
||
### 2. GLOBAL_BAN Config Propagates Punishments When True
|
||
| Item | Status | Evidence |
|
||
|---|---|---|
|
||
| Config field present | ✅ | `GroupsConfig.global_ban: bool` with `default_global_ban() -> false` |
|
||
| Propagation logic | ✅ | `propagate_punishment()` in `cgcx-bot/src/main.rs:2253` — early returns if `!global_ban` |
|
||
| Target chats | ✅ | Admin groups + review groups + all active forward destination chats |
|
||
| Bot admin check | ✅ | Skips chats where the bot is not an admin (`is_admin`) |
|
||
| Supported actions | ✅ | `ban`, `mute`, `kick` with duration propagation |
|
||
| DB recording | ✅ | Propagated punishments are inserted into `punishments` table per chat |
|
||
|
||
---
|
||
|
||
### 3. Author Visibility Toggle Works During Upload
|
||
| Item | Status | Evidence |
|
||
|---|---|---|
|
||
| Upload option exists | ✅ | `UploadOptions.show_author: bool`, default `true` |
|
||
| Bot toggle callback | ✅ | `"v1:opt:toggle_author"` → flips `show_author`, refreshes options message |
|
||
| Stored in DB | ✅ | `contents.show_author` INTEGER NOT NULL DEFAULT 1 (migration 005) |
|
||
| Respected in upload result | ✅ | `main.rs:1346` — `author_text` only shown when `options.show_author` is true |
|
||
| Respected in forward post | ✅ | `main.rs:1937` — `author_line` respects `content.show_author` |
|
||
|
||
---
|
||
|
||
### 4. Metadata (date, size, author) Displays Correctly on View Page
|
||
| Item | Status | Evidence |
|
||
|---|---|---|
|
||
| Date | ✅ | `new Date(metadata.created_at).toLocaleString()` in `ViewContent.svelte:121` |
|
||
| Size | ✅ | `formatSize(metadata.total_size)` in `ViewContent.svelte:122` |
|
||
| Author (visible) | ✅ | `metadata.author` rendered as `@username [user_id]` with Telegram link when `show_author=true` |
|
||
| Author (hidden) | ✅ | Server returns `author: null` when `show_author=false` (`main.rs:534`) |
|
||
| Server-side gating | ✅ | `get_metadata` only resolves `AuthorInfo` when `content.show_author` is true |
|
||
| View count / max views | ✅ | `metadata.current_views / metadata.max_views` displayed conditionally |
|
||
|
||
---
|
||
|
||
### 5. Deduplication Reuses Existing Encrypted Files for Identical Uploads
|
||
| Item | Status | Evidence |
|
||
|---|---|---|
|
||
| Hashing | ✅ | `blake3::Hasher` computes `plaintext_hash` during ingestion |
|
||
| Reuse logic | ✅ | `FilePipeline::ingest_file` checks `find_active_by_plaintext_hash` (line 138) |
|
||
| Ref count increment | ✅ | `increment_ref_count(&existing.content_id, existing.file_index)` called |
|
||
| New entry created | ✅ | A new `ContentFile` row is inserted pointing to the existing `stored_path` |
|
||
| Temp file dropped | ✅ | `drop(named_temp)` on dedup path avoids writing duplicate ciphertext |
|
||
| Migration present | ✅ | `006_dedup.sql` adds `plaintext_hash` and `ref_count` columns |
|
||
|
||
---
|
||
|
||
### 6. Hash Blacklist Blocks Re-Uploads of Banned Content
|
||
| Item | Status | Evidence |
|
||
|---|---|---|
|
||
| Blacklist table | ✅ | `hash_blacklist` table created by migration 007 |
|
||
| Check before store | ✅ | `HashBlacklistRepo::contains(hash_bytes)` called in `ingest_file` before dedup (line 132) |
|
||
| Error type | ✅ | Returns `CgcxError::BlockedHash` when blacklisted |
|
||
| Moderator API | ✅ | `HashBlacklistRepo::insert(hash, reason)` available for adding entries |
|
||
|
||
---
|
||
|
||
### 7. Username Changes Are Logged to Configured JSON File
|
||
| Item | Status | Evidence |
|
||
|---|---|---|
|
||
| Config path | ✅ | `Config.uname_changes_path`, default `"data/uname_changes.json"` |
|
||
| Tracking trigger | ✅ | `UserRepo::ensure_exists` logs when `old_username != new_username` |
|
||
| Log format | ✅ | JSON line per change: `{timestamp, user_id, old_username, new_username, chat_id}` |
|
||
| File mode | ✅ | `OpenOptions::new().create(true).append(true)` |
|
||
| Called on every interaction | ✅ | Bot calls `user_repo.ensure_exists(..., Some(&uname_changes_path))` on messages and callbacks |
|
||
|
||
---
|
||
|
||
## Blockers / Risks
|
||
|
||
1. **Zero Test Coverage**
|
||
- No unit or integration tests exist in any crate. All verification above is static code review.
|
||
- **Recommendation:** Add at least integration tests for `FilePipeline::ingest_file` (dedup + blacklist paths) and the server metadata/password flows.
|
||
|
||
2. **View Count Increment Location (Minor)**
|
||
- `serve_file` and `serve_raw_file` both increment views. A HEAD request or 304 Not Modified response correctly skips the increment.
|
||
- Range requests (`is_range`) also skip increment. This is intentional but worth noting.
|
||
|
||
3. **Chunk Size Warning (Non-blocking)**
|
||
- Frontend build warns about a >500KB JS chunk. This is a performance consideration, not a functional blocker.
|
||
|
||
4. **Clippy Cleanliness**
|
||
- Workspace now passes `clippy -- -D warnings`. This was not true before this QA run.
|
||
|
||
---
|
||
|
||
## Summary
|
||
|
||
All requested features (review buttons, GLOBAL_BAN propagation, author toggle, metadata display, deduplication, hash blacklist, username tracking) are **implemented and appear correct** based on static analysis. The workspace compiles, tests pass (trivially), clippy is clean, and the frontend builds successfully. The primary risk is the complete absence of automated tests.
|