fix: add catch to avoid return undefined types #674
Conversation
Signed-off-by: Tomás Castillo <[email protected]>
src/components/forms/room-form.js
Outdated
| }; | ||
| }); | ||
| const roomAttributes = entity.attributes | ||
| .map((attr) => { |
There was a problem hiding this comment.
@tomrndom lests change the approach using flat map and reduce
const typeById = new Map(attributeTypes.map(t => [t.id, t]));
const roomAttributes = (entity.attributes ?? []).reduce((acc, attr) => {
const type = typeById.get(attr.type_id);
if (!type) return acc;
acc.push({
...attr,
label: `${type.type}: ${attr.value}`,
type_name: type.type,
});
return acc;
}, []);
Signed-off-by: Tomás Castillo <[email protected]>
📝 WalkthroughWalkthroughRefactors room attribute resolution in the room form component: builds a map of attribute types and reduces over Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)src/components/forms/room-form.js (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
🔇 Additional comments (2)
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
🤖 Fix all issues with AI agents
In @src/components/forms/room-form.js:
- Around line 169-180: attributeTypes is referenced but not declared, causing a
ReferenceError; before creating typeById in the render (or top of the component
function), declare and initialize attributeTypes from the component inputs
(e.g., this.props.attributeTypes or props.attributeTypes) and default it to an
empty array to safe-guard against undefined so the existing typeById and
roomAttributes logic can run without crashing.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/forms/room-form.js
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/forms/room-form.js (1)
src/reducers/summits/current-summit-reducer.js (5)
attributeTypes(490-493)attributeTypes(504-507)attributeTypes(519-522)attributeTypes(543-546)entity(251-251)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
Signed-off-by: Tomás Castillo <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/forms/room-form.js (1)
116-119: Apply the same null-safety pattern toqueryAttributes.The
queryAttributesmethod has a similar issue:findat line 117 can returnundefined, and line 118 will then throw a TypeError when accessingtype.type.🐛 Proposed fix using the same Map-based pattern
queryAttributes(input, callback) { const { currentSummit } = this.props; const attributeTypes = currentSummit.meeting_booking_room_allowed_attributes; + const typeById = new Map(attributeTypes.map((t) => [t.id, t])); let attributes = []; attributeTypes.forEach((type) => { attributes = [...attributes, ...type.values]; }); - attributes = attributes.map((attr) => { - const type = attributeTypes.find((at) => at.id === attr.type_id); - return { ...attr, label: `${type.type}: ${attr.value}` }; - }); + attributes = attributes.reduce((acc, attr) => { + const type = typeById.get(attr.type_id); + if (!type) return acc; + acc.push({ ...attr, label: `${type.type}: ${attr.value}` }); + return acc; + }, []); attributes = attributes.filter( (at) => at.label.toLowerCase().indexOf(input.toLowerCase()) !== -1 ); callback(attributes); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/forms/room-form.js
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/forms/room-form.js (1)
src/reducers/summits/current-summit-reducer.js (4)
attributeTypes(490-493)attributeTypes(504-507)attributeTypes(519-522)attributeTypes(543-546)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (2)
src/components/forms/room-form.js (2)
172-183: Excellent implementation following the suggested pattern!The Map-based lookup with reduce provides:
- Performance: O(1) type lookups vs O(n) repeated
findcalls- Robustness: Null-safe handling with
entity.attributes ?? []- Safety: Gracefully skips attributes when type is not found instead of throwing errors
The output shape is preserved, making this a safe refactor.
169-170: LGTM! Critical fix addresses the ReferenceError.This declaration resolves the critical issue where
attributeTypeswas referenced without being defined, which would have caused a ReferenceError at runtime. The subsequent Map-based lookup pattern (lines 172-183) properly handles missing attribute types with the safety check.
ref: https://app.clickup.com/t/86b6z5qrd
Signed-off-by: Tomás Castillo [email protected]
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.