fix: hide speaker input search on speakers promocodes not saved#806
fix: hide speaker input search on speakers promocodes not saved#806
Conversation
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
📝 WalkthroughWalkthroughA single component file underwent refactoring that includes converting a callback to an arrow function, removing unused method parameters, eliminating unnecessary property destructuring, and adding conditional rendering logic for the FreeTextSearch component based on entity ID values. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/forms/promocode-form/forms/speakers-base-pc-form.js`:
- Around line 135-146: Replace the numeric comparison entity.id > 0 with a
presence check so string IDs won't hide the search; change the conditional
around the FreeTextSearch (the block rendering FreeTextSearch that uses
value={term ?? ""}, onSearch={handleSearch}) to use a boolean presence check
like !!entity.id (or simply if (entity.id)) to match the check used earlier in
the file.
| {entity.id > 0 && ( | ||
| <div className="col-md-6" style={{ zIndex: 0 }}> | ||
| <FreeTextSearch | ||
| value={term ?? ""} | ||
| placeholder={T.translate( | ||
| "edit_promo_code.placeholders.search_speakers" | ||
| )} | ||
| onSearch={handleSearch} | ||
| preventEvents | ||
| /> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the speakers-base-pc-form.js file to understand context
fd -n "speakers-base-pc-form" --type fRepository: fntechgit/summit-admin
Length of output: 296
🏁 Script executed:
# Search for id prop-type or type declarations in the codebase
rg -n -C3 --type=js --type=jsx --type=ts --type=tsx '\bid\s*:\s*(PropTypes\.)?(number|string|Number|String)|\bid\s*[:=]\s*(Number|String|PropTypes\.number|PropTypes\.string)\b' | head -80Repository: fntechgit/summit-admin
Length of output: 93
🏁 Script executed:
# Find numeric id guards similar to the one in question
rg -n -C2 --type=js --type=jsx --type=ts --type=tsx '\.id\s*>\s*0' | head -60Repository: fntechgit/summit-admin
Length of output: 93
🏁 Script executed:
# Also check for entity/model definitions to understand id type
rg -n -C3 --type=js --type=jsx --type=ts --type=tsx 'entity\s*[:=].*id|id\s*:\s*.*(?:number|string|Number|String|uuid|UUID)' | head -80Repository: fntechgit/summit-admin
Length of output: 93
🏁 Script executed:
# Find the speakers-base-pc-form.js file
fd "speakers-base-pc-form" --type fRepository: fntechgit/summit-admin
Length of output: 132
🏁 Script executed:
# Search for id prop-type or type declarations in the codebase
rg -n -C3 '\bid\s*:\s*(PropTypes\.)?(number|string|Number|String)|\bid\s*[:=]\s*(Number|String|PropTypes\.number|PropTypes\.string)' --type=js --type=ts 2>/dev/null | head -100Repository: fntechgit/summit-admin
Length of output: 5728
🏁 Script executed:
# Find numeric id guards similar to the one in question
rg -n -C2 '\.id\s*>\s*0' --type=js --type=ts 2>/dev/null | head -60Repository: fntechgit/summit-admin
Length of output: 4358
🏁 Script executed:
# Read the speakers-base-pc-form.js file to understand context around line 135-146
fd "speakers-base-pc-form.js" --type f -x cat -n {}Repository: fntechgit/summit-admin
Length of output: 6749
🏁 Script executed:
# Search for entity model/interface definitions
rg -n -C3 'entity.*id|interface.*entity|type.*entity' --type=js --type=ts 2>/dev/null | head -100Repository: fntechgit/summit-admin
Length of output: 7071
Use a presence check for entity.id instead of numeric comparison.
Line 135 uses entity.id > 0, but IDs in this codebase can be strings (e.g., PropTypes.string appears in multiple components). If a persisted entity has a string ID, this check would fail and hide the search component. Line 26 in the same file already uses the safer if (entity.id) pattern. Use !!entity.id for consistency and to handle both numeric and string IDs.
Suggested change
- {entity.id > 0 && (
+ {!!entity.id && (📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {entity.id > 0 && ( | |
| <div className="col-md-6" style={{ zIndex: 0 }}> | |
| <FreeTextSearch | |
| value={term ?? ""} | |
| placeholder={T.translate( | |
| "edit_promo_code.placeholders.search_speakers" | |
| )} | |
| onSearch={handleSearch} | |
| preventEvents | |
| /> | |
| </div> | |
| )} | |
| {!!entity.id && ( | |
| <div className="col-md-6" style={{ zIndex: 0 }}> | |
| <FreeTextSearch | |
| value={term ?? ""} | |
| placeholder={T.translate( | |
| "edit_promo_code.placeholders.search_speakers" | |
| )} | |
| onSearch={handleSearch} | |
| preventEvents | |
| /> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/forms/promocode-form/forms/speakers-base-pc-form.js` around
lines 135 - 146, Replace the numeric comparison entity.id > 0 with a presence
check so string IDs won't hide the search; change the conditional around the
FreeTextSearch (the block rendering FreeTextSearch that uses value={term ?? ""},
onSearch={handleSearch}) to use a boolean presence check like !!entity.id (or
simply if (entity.id)) to match the check used earlier in the file.
1 participant
ref: https://app.clickup.com/t/86b8mpbev
Signed-off-by: Tomás Castillo tcastilloboireau@gmail.com
Summary by CodeRabbit
Release Notes
Refactor
Bug Fixes