refactor(activists): organizer columns and fill in undefined booleans#346
Merged
refactor(activists): organizer columns and fill in undefined booleans#346
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughReplace array-based column lookups with a name→definition map, move column-selection logic into a new module, add Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Activists UI
participant ColDefs as ColumnDefinitions (map)
participant Query as Query Builder
participant API as ApiClient
participant Processor as Response Processor
UI->>ColDefs: Read COLUMN_DEFINITION_BY_NAME, GROUPED_COLUMNS_BY_CATEGORY
UI->>Query: Build query (normalizeColumnsForFilters, columnsForNewFilters)
Query->>API: searchActivists(request)
API->>Processor: parse JSON -> fillActivistBlankFields using BLANK_TO_ZERO_FIELDS / BLANK_TO_FALSE_FIELDS
Processor->>UI: return QueryActivistResult with mutated activists
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
alexsapps
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend-v2/src/app/`(authed)/activists/column-definitions.ts:
- Around line 457-466: The grouping uses spread on each iteration causing
unnecessary allocations; update the GROUPED_COLUMNS_BY_CATEGORY IIFE to retrieve
the array for a category from the Map (GROUPED_COLUMNS_BY_CATEGORY) or create
and set a new array when missing, then call .push(column) on that array instead
of setting a new array via [...existing, column]; keep the Map type and final
return intact and reference COLUMN_DEFINITIONS and column.category/column to
locate the loop to modify.
In `@frontend-v2/src/app/`(authed)/activists/column-selection.ts:
- Around line 5-20: The comment above FILTER_COLUMN_MAP is missing mention of
the prospect filter; either add prospect to the excluded list in the comment (if
there is no single corresponding column) or add a mapping entry in
FILTER_COLUMN_MAP for prospect (using the correct ActivistColumnName value from
ActivistColumnName / FilterState) so the code and comment stay consistent—update
the comment text or add the prospect: 'prospect' mapping in the
FILTER_COLUMN_MAP accordingly.
In `@frontend-v2/src/lib/api.ts`:
- Line 605: The function fillActivistBlankNumericFieldsWithZero now normalizes
both numeric and boolean blanks, so rename it to a clearer name (e.g.,
normalizeActivistBlankFields or fillActivistBlankFields) and update all
references: rename the function declaration and any exports, imports, and
callers that reference fillActivistBlankNumericFieldsWithZero (search for that
symbol across the repo), and update JSDoc/type annotations and tests to reflect
the new name and intent; ensure you run typechecks and adjust any public API
docs where the old name appears.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8ce2e465-1241-4563-8d1a-ee4f4e5c6da7
📒 Files selected for processing (11)
frontend-v2/src/app/(authed)/activists/activists-table.test.tsxfrontend-v2/src/app/(authed)/activists/activists-table.tsxfrontend-v2/src/app/(authed)/activists/column-definitions.tsfrontend-v2/src/app/(authed)/activists/column-selection.tsfrontend-v2/src/app/(authed)/activists/column-selector.tsxfrontend-v2/src/app/(authed)/activists/filter-api-query.tsfrontend-v2/src/app/(authed)/activists/format-value.tsfrontend-v2/src/app/(authed)/activists/search-params.tsfrontend-v2/src/app/(authed)/activists/sort-selector.tsxfrontend-v2/src/app/(authed)/activists/use-activist-query-state.tsfrontend-v2/src/lib/api.ts
alexsapps
force-pushed
the
alex/column-refactor
branch
from
a8ba584 to
c9c41f5
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend-v2/src/app/`(authed)/activists/column-definitions.ts:
- Around line 441-443: COLUMN_DEFINITION_BY_NAME is being asserted as
Record<ActivistColumnName, ColumnDefinition> but ActivistColumnName (derived
from Object.keys(ActivistJSON.shape)) includes "chapter_id" which is not present
in COLUMN_DEFINITIONS, so the assertion is lying; fix by either adding a
ColumnDefinition for "chapter_id" to COLUMN_DEFINITIONS (with appropriate
display/formatter/width) or remove "chapter_id" from the ActivistJSON.shape (or
from the ActivistColumnName union) if it should not be a visible column; update
the code where COLUMN_DEFINITIONS and ActivistJSON.shape are defined to keep
them in sync so COLUMN_DEFINITION_BY_NAME genuinely contains every
ActivistColumnName.
In `@frontend-v2/src/lib/api.ts`:
- Around line 8-11: Move the BLANK_TO_ZERO_FIELDS and BLANK_TO_FALSE_FIELDS
constants out of the application layer and into the API layer by adding them to
the API activists module where ActivistJSON is defined (e.g.,
`@/lib/api/activists.ts`), then update imports in frontend-v2/src/lib/api.ts to
import those constants from the new API module instead of from
`@/app/`(authed)/activists/column-definitions; ensure the constants are exported
from the activists API module and used by any code that normalizes ActivistJSON
objects so you preserve behavior while eliminating the lib -> app dependency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 680bc0e9-6f09-4829-bcbc-c3abb2c572e5
📒 Files selected for processing (3)
frontend-v2/src/app/(authed)/activists/column-definitions.tsfrontend-v2/src/app/(authed)/activists/column-selection.tsfrontend-v2/src/lib/api.ts
alexsapps
force-pushed
the
alex/column-refactor
branch
from
c9c41f5 to
cab30d9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit