2026 Code Clean Up #487

New issue

Open

opened 2026-01-22 17:31:07 +00:00 by Vylpes · 0 comments

Vylpes commented

2026-01-22 17:31:07 +00:00

Owner

Copy link

Scan the repository for areas of code which can be cleaned up and perhaps written a bit cleaner. Identify any major areas not covered by testing or documentation which should be.

Card Drop cleanup + coverage checklist

Actionable items from a 2026 repository scan. Check items off as they are completed.

Code cleanup

Rename misspelled GuildMemebrUpdate to GuildMemberUpdate
- Files: src/contracts/EventExecutors.ts, src/client/client.ts, src/client/util.ts
- Update registration and event-load wiring consistently so member-update handlers bind correctly.
Await async command / button / dropdown handlers in dispatchers
- Files: src/client/interactionCreate/Button.ts, src/client/interactionCreate/ChatInputCommand.ts, src/client/interactionCreate/StringDropdown.ts
- Today handlers are fired without await, so rejected promises bypass the surrounding try/catch.
- Change to await item.Event.execute(...) / await itemToUse.Command.execute(...).
- Improve error logging (avoid casting errors with e as string; use AppLogger.CatchError or similar).
Throw Error objects instead of bare strings
- Files: src/bot.ts, src/client/appLogger.ts
- Convert throw "..." to throw new Error("...") and keep logging consistent.
Replace loose equality (== / !=) with strict equality (=== / !==)
- Notable files: src/helpers/DropHelpers/GetUnclaimedCardsHelper.ts, src/helpers/InventoryHelper.ts, src/commands/sacrifice.ts, src/helpers/SeriesHelper.ts, src/helpers/TimerHelper.ts, and related helpers/commands that compare quantities/IDs.
- Optionally enable/confirm eqeqeq in ESLint, then fix remaining violations.
Stop mutating source arrays during pagination
- File: src/helpers/SeriesHelper.ts
- Replace splice page selection with slice so cloned card lists are not mutated as a side effect.
Stop swallowing image-generation errors
- File: src/helpers/ImageHelper.ts
- The empty catch with a TODO currently drops failures. Capture the error, log it (or rethrow), and cover failure modes in tests.
Fix AppBaseEntity.FetchOneById relations default
- File: src/contracts/AppBaseEntity.ts
- Uses relations || {}; TypeORM expects an array. Default to relations || [] (same pattern as FetchAll).
Harden timer tick execution
- File: src/helpers/TimerHelper.ts
- Use strict equality for timer ID lookup.
- Await / safely handle async onTick callbacks and log tick failures.
Add authentication to the reload webhook
- File: src/webhooks.ts (POST /api/reload-db)
- Require a shared secret/token header (or similar), document the env var, and add a lightweight test.
Project metadata / dependency cleanup
- File: package.json
- Move jest (and related test packages if appropriate) from dependencies to devDependencies.
- Fix malformed bugs.url (https//... → https://...).

Testing gaps

Current Jest coverage is concentrated on drop/multidrop/effects commands, a subset of button events, PurgeClaims, and a few helpers. Priority gaps:

Add helper tests for uncovered helpers
- src/helpers/InventoryHelper.ts
- src/helpers/CardSearchHelper.ts
- src/helpers/SeriesHelper.ts
- src/helpers/DropHelpers/GetUnclaimedCardsHelper.ts
- src/helpers/ImageHelper.ts
Add command tests for untested slash commands
- Missing or lightly covered: daily, gdrivesync, give, inventory, resync, sacrifice, series, stats, trade, view, about, id, and stage/*.
Add button-event tests for untested handlers
- Missing: inventory, sacrifice, series, trade, view.
Add string-dropdown coverage
- src/stringDropdowns/Inventory.ts and dispatch path in src/client/interactionCreate/StringDropdown.ts.
Add webhook + middleware tests
- src/webhooks.ts
- src/client/interactionCreate/middleware/NewUserDiscovery.ts
Add timer coverage for GiveCurrency
- src/timers/GiveCurrency.ts (only PurgeClaims is tested today).
Assert dispatcher error-handling after awaiting handlers
- Ensure rejected promises are caught and users receive a consistent error reply.

Documentation gaps

Expand README with developer ops
- Document yarn test / yarn lint.
- Clarify DB migration workflow beyond a brief db:up mention.
- Document POST /api/reload-db (and auth once added).
Document how to add commands / button events / dropdown events
- Point at src/registry.ts and the interaction-create dispatchers so contributors know where to register new handlers.
Cross-link existing docs to testing / env expectations
- docs/cards.md, docs/google-drive-sync.md, docs/logger.md should reference where env vars live and what tests are expected when changing those areas.

Suggested order of attack

Fix dispatcher await + error logging (correctness).
Fix GuildMemebrUpdate typo and AppBaseEntity relations default.
Replace splice pagination and empty catch in ImageHelper.
Secure /api/reload-db and document it.
Expand tests for helpers/dispatchers first, then high-traffic commands (inventory, sacrifice, trade, series, view).
Fill README / contributor docs last once behaviour is stabilized.

Scan the repository for areas of code which can be cleaned up and perhaps written a bit cleaner. Identify any major areas not covered by testing or documentation which should be. --- # Card Drop cleanup + coverage checklist Actionable items from a 2026 repository scan. Check items off as they are completed. ## Code cleanup - [ ] **Rename misspelled `GuildMemebrUpdate` to `GuildMemberUpdate`** - Files: `src/contracts/EventExecutors.ts`, `src/client/client.ts`, `src/client/util.ts` - Update registration and event-load wiring consistently so member-update handlers bind correctly. - [ ] **Await async command / button / dropdown handlers in dispatchers** - Files: `src/client/interactionCreate/Button.ts`, `src/client/interactionCreate/ChatInputCommand.ts`, `src/client/interactionCreate/StringDropdown.ts` - Today handlers are fired without `await`, so rejected promises bypass the surrounding `try/catch`. - Change to `await item.Event.execute(...)` / `await itemToUse.Command.execute(...)`. - Improve error logging (avoid casting errors with `e as string`; use `AppLogger.CatchError` or similar). - [ ] **Throw `Error` objects instead of bare strings** - Files: `src/bot.ts`, `src/client/appLogger.ts` - Convert `throw "..."` to `throw new Error("...")` and keep logging consistent. - [ ] **Replace loose equality (`==` / `!=`) with strict equality (`===` / `!==`)** - Notable files: `src/helpers/DropHelpers/GetUnclaimedCardsHelper.ts`, `src/helpers/InventoryHelper.ts`, `src/commands/sacrifice.ts`, `src/helpers/SeriesHelper.ts`, `src/helpers/TimerHelper.ts`, and related helpers/commands that compare quantities/IDs. - Optionally enable/confirm `eqeqeq` in ESLint, then fix remaining violations. - [ ] **Stop mutating source arrays during pagination** - File: `src/helpers/SeriesHelper.ts` - Replace `splice` page selection with `slice` so cloned card lists are not mutated as a side effect. - [ ] **Stop swallowing image-generation errors** - File: `src/helpers/ImageHelper.ts` - The empty `catch` with a TODO currently drops failures. Capture the error, log it (or rethrow), and cover failure modes in tests. - [ ] **Fix `AppBaseEntity.FetchOneById` relations default** - File: `src/contracts/AppBaseEntity.ts` - Uses `relations || {}`; TypeORM expects an array. Default to `relations || []` (same pattern as `FetchAll`). - [ ] **Harden timer tick execution** - File: `src/helpers/TimerHelper.ts` - Use strict equality for timer ID lookup. - Await / safely handle async `onTick` callbacks and log tick failures. - [ ] **Add authentication to the reload webhook** - File: `src/webhooks.ts` (`POST /api/reload-db`) - Require a shared secret/token header (or similar), document the env var, and add a lightweight test. - [ ] **Project metadata / dependency cleanup** - File: `package.json` - Move `jest` (and related test packages if appropriate) from `dependencies` to `devDependencies`. - Fix malformed `bugs.url` (`https//...` → `https://...`). ## Testing gaps Current Jest coverage is concentrated on drop/multidrop/effects commands, a subset of button events, `PurgeClaims`, and a few helpers. Priority gaps: - [ ] **Add helper tests for uncovered helpers** - `src/helpers/InventoryHelper.ts` - `src/helpers/CardSearchHelper.ts` - `src/helpers/SeriesHelper.ts` - `src/helpers/DropHelpers/GetUnclaimedCardsHelper.ts` - `src/helpers/ImageHelper.ts` - [ ] **Add command tests for untested slash commands** - Missing or lightly covered: `daily`, `gdrivesync`, `give`, `inventory`, `resync`, `sacrifice`, `series`, `stats`, `trade`, `view`, `about`, `id`, and `stage/*`. - [ ] **Add button-event tests for untested handlers** - Missing: `inventory`, `sacrifice`, `series`, `trade`, `view`. - [ ] **Add string-dropdown coverage** - `src/stringDropdowns/Inventory.ts` and dispatch path in `src/client/interactionCreate/StringDropdown.ts`. - [ ] **Add webhook + middleware tests** - `src/webhooks.ts` - `src/client/interactionCreate/middleware/NewUserDiscovery.ts` - [ ] **Add timer coverage for `GiveCurrency`** - `src/timers/GiveCurrency.ts` (only `PurgeClaims` is tested today). - [ ] **Assert dispatcher error-handling after awaiting handlers** - Ensure rejected promises are caught and users receive a consistent error reply. ## Documentation gaps - [ ] **Expand README with developer ops** - Document `yarn test` / `yarn lint`. - Clarify DB migration workflow beyond a brief `db:up` mention. - Document `POST /api/reload-db` (and auth once added). - [ ] **Document how to add commands / button events / dropdown events** - Point at `src/registry.ts` and the interaction-create dispatchers so contributors know where to register new handlers. - [ ] **Cross-link existing docs to testing / env expectations** - `docs/cards.md`, `docs/google-drive-sync.md`, `docs/logger.md` should reference where env vars live and what tests are expected when changing those areas. ## Suggested order of attack 1. Fix dispatcher `await` + error logging (correctness). 2. Fix `GuildMemebrUpdate` typo and `AppBaseEntity` relations default. 3. Replace `splice` pagination and empty catch in `ImageHelper`. 4. Secure `/api/reload-db` and document it. 5. Expand tests for helpers/dispatchers first, then high-traffic commands (`inventory`, `sacrifice`, `trade`, `series`, `view`). 6. Fill README / contributor docs last once behaviour is stabilized.

Vylpes added this to the 0.12.0 milestone

2026-01-22 17:31:57 +00:00

Vylpes added the

Rows
Columns