Feature: Field Extraction #196
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds a pattern-driven field extraction system for calendar events, rewrites README with extraction docs, replaces extractMeetingInfo with extractFields, introduces fieldExtraction settings and defaults, updates settings UI to manage patterns, migrates legacy settings, propagates extractedFields to events, and adjusts tests accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Plugin as Plugin (main.ts)
participant SettingsStore as Settings (ICSSettings)
participant UI as Settings UI (ICSSettingsTab)
participant Extractor as extractFields (icalUtils)
rect rgba(200,230,255,0.18)
Plugin->>SettingsStore: loadSettings()
SettingsStore-->>Plugin: settings (migrated if needed)
Plugin->>SettingsStore: save migrated defaults (if applied)
end
User->>UI: open settings
UI-->>User: render fieldExtraction UI
User->>UI: add/edit/delete patterns
UI->>SettingsStore: save fieldExtraction changes
rect rgba(230,255,230,0.18)
Plugin->>Extractor: extractFields(event, patterns)
Extractor->>Extractor: sort by priority, apply contains/regex, collect matches
Extractor-->>Plugin: extractedFields (Record<string,string[]>)
Plugin->>Plugin: derive callUrl/callType (back-compat), attach to IEvent
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (8)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/icalUtils.ts (1)
15-20: Avoid mutating caller-supplied pattern arrays
patterns.sort(...)mutates the array that was passed in, which means every call reordersthis.data.videoCallExtraction.patterns(and the exported defaults when used directly), creating surprising statefulness. Copy the array before sorting so extraction stays side-effect free.- const sortedPatterns = patterns.sort((a, b) => a.priority - b.priority); + const sortedPatterns = [...patterns].sort((a, b) => a.priority - b.priority);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.md(1 hunks)src/icalUtils.ts(1 hunks)src/main.ts(3 hunks)src/settings/ICSSettings.ts(2 hunks)src/settings/ICSSettingsTab.ts(1 hunks)tests/icalUtils.test.ts(6 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md
164-164: Hard tabs
Column: 1
(MD010, no-hard-tabs)
165-165: Hard tabs
Column: 1
(MD010, no-hard-tabs)
166-166: Hard tabs
Column: 1
(MD010, no-hard-tabs)
167-167: Hard tabs
Column: 1
(MD010, no-hard-tabs)
168-168: Hard tabs
Column: 1
(MD010, no-hard-tabs)
169-169: Hard tabs
Column: 1
(MD010, no-hard-tabs)
170-170: Hard tabs
Column: 1
(MD010, no-hard-tabs)
171-171: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🪛 ast-grep (0.39.5)
src/icalUtils.ts
[warning] 60-60: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(pattern.pattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
src/settings/ICSSettingsTab.ts
[warning] 765-765: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(this.pattern.pattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 786-786: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(this.pattern.pattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 323-325: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: divSponsor.innerHTML = <br/><hr/>A scratch my own itch project by <a href="https://muness.com/" target='_blank'>muness</a>.<br/> <a href='https://www.buymeacoffee.com/muness' target='_blank'><img height="36" src='https://cdn.buymeacoffee.com/uploads/profile_pictures/default/79D6B5/MC.png' border='0' alt='Buy Me a Book' /></a>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 323-325: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: divSponsor.innerHTML = <br/><hr/>A scratch my own itch project by <a href="https://muness.com/" target='_blank'>muness</a>.<br/> <a href='https://www.buymeacoffee.com/muness' target='_blank'><img height="36" src='https://cdn.buymeacoffee.com/uploads/profile_pictures/default/79D6B5/MC.png' border='0' alt='Buy Me a Book' /></a>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
🔇 Additional comments (3)
tests/icalUtils.test.ts (1)
104-145: Tests now exercise configurable pattern flowPassing
DEFAULT_VIDEO_CALL_PATTERNSintoextractMeetingInfokeeps the spec aligned with the new signature and validates the default pattern set end-to-end. Thanks for wiring this up.README.md (2)
162-171: Replace hard tabs in Full Calendar exampleLines [164-171] contain hard tabs, which trip the markdownlint
MD010rule in the static analysis hints. Swapping them for spaces keeps the lint pipeline green without changing the example’s intent. Apply something like:- const start = moment.unix(event.utime) - const [endHours, endMinutes] = event.endTime.split(":") - return { - start: start.toDate(), - end: start.set({hour: endHours, minute: endMinutes}).toDate(), - title: event.summary, - } - } + const start = moment.unix(event.utime) + const [endHours, endMinutes] = event.endTime.split(":") + return { + start: start.toDate(), + end: start.set({hour: endHours, minute: endMinutes}).toDate(), + title: event.summary, + } + }[ suggest_optional_refactor ]
Based on static analysis hints
223-223: Correct “Obdisian” typoLine [223] introduces a misspelling of “Obsidian”. Suggested fix:
-5. copy the `dist/main.js` (or `dist/main-debug.js` if you want the un-minified version) to your Obdisian vault plugin folder (`cp dist/main.js <vault>/.obsidian/plugins/ics/main.js`). +5. copy the `dist/main.js` (or `dist/main-debug.js` if you want the un-minified version) to your Obsidian vault plugin folder (`cp dist/main.js <vault>/.obsidian/plugins/ics/main.js`).Likely an incorrect or invalid review comment.
|
|
||
| See [advanced Templated usage example](https://github.com/cloud-atlas-ai/obsidian-ics/discussions/74#discussion-5779931) for an example that demonstrates more features. | ||
|
|
||
| You can see the available fields an the [Event interface](https://github.com/cloud-atlas-ai/obsidian-ics/blob/master/src/IEvent.ts). |
There was a problem hiding this comment.
Fix typo: “an the” → “in the”
Line [152] reads “an the”. Small doc polish, but worth fixing:
-You can see the available fields an the [Event interface](https://github.com/cloud-atlas-ai/obsidian-ics/blob/master/src/IEvent.ts).
+You can see the available fields in the [Event interface](https://github.com/cloud-atlas-ai/obsidian-ics/blob/master/src/IEvent.ts).📝 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.
| You can see the available fields an the [Event interface](https://github.com/cloud-atlas-ai/obsidian-ics/blob/master/src/IEvent.ts). | |
| You can see the available fields in the [Event interface](https://github.com/cloud-atlas-ai/obsidian-ics/blob/master/src/IEvent.ts). |
🤖 Prompt for AI Agents
In README.md around line 152, there is a typo "an the" in the sentence; replace
"an the" with "in the" so the line reads "You can see the available fields in
the [Event
interface](https://github.com/cloud-atlas-ai/obsidian-ics/blob/master/src/IEvent.ts)."
src/main.ts
Outdated
| this.data.videoCallExtraction = { | ||
| enabled: true, | ||
| patterns: [...DEFAULT_VIDEO_CALL_PATTERNS] | ||
| }; | ||
| needsSave = true; | ||
| } else { | ||
| // Ensure patterns array exists | ||
| if (!this.data.videoCallExtraction.patterns) { | ||
| this.data.videoCallExtraction.patterns = [...DEFAULT_VIDEO_CALL_PATTERNS]; | ||
| needsSave = true; |
There was a problem hiding this comment.
Deep clone video-call defaults during migration
This migration still copies DEFAULT_VIDEO_CALL_PATTERNS by reference—[...DEFAULT_VIDEO_CALL_PATTERNS] creates a new array but keeps the same pattern objects. Editing or reordering patterns in the settings UI will therefore mutate the exported defaults, so “reset to defaults” no longer restores the original values and those mutations get saved back as if they were defaults. Please clone the pattern objects before storing them.
- patterns: [...DEFAULT_VIDEO_CALL_PATTERNS]
+ patterns: DEFAULT_VIDEO_CALL_PATTERNS.map(pattern => ({ ...pattern }))The same adjustment is needed in every branch where we hydrate videoCallExtraction.patterns.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main.ts around lines 269 to 278, the migration copies
DEFAULT_VIDEO_CALL_PATTERNS by shallow-cloning the array which keeps references
to the same pattern objects; change assignments so you deep-clone the pattern
objects (e.g. map over DEFAULT_VIDEO_CALL_PATTERNS and create new objects for
each pattern) before storing them into this.data.videoCallExtraction.patterns,
and apply the same deep-clone approach to every other branch in this file where
videoCallExtraction.patterns is hydrated during migration.
src/settings/ICSSettings.ts
Outdated
| videoCallExtraction: { | ||
| enabled: true, | ||
| patterns: DEFAULT_VIDEO_CALL_PATTERNS | ||
| } |
There was a problem hiding this comment.
Prevent default video-call patterns from being mutated at runtime
DEFAULT_SETTINGS.videoCallExtraction.patterns currently points straight at DEFAULT_VIDEO_CALL_PATTERNS. Because loadSettings does a shallow Object.assign, the plugin state shares that same array/object. As soon as we call extractMeetingInfo (which sorts in place) or the user edits/reorders a pattern, we mutate the exported defaults. “Reset to defaults” will then reapply the already-mutated data, and future migrations treat those mutations as canonical defaults. Please deep-clone the defaults before storing them (and mirror that change in the migration code so every entry is copied).
- patterns: DEFAULT_VIDEO_CALL_PATTERNS
+ patterns: DEFAULT_VIDEO_CALL_PATTERNS.map(pattern => ({ ...pattern }))📝 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.
| videoCallExtraction: { | |
| enabled: true, | |
| patterns: DEFAULT_VIDEO_CALL_PATTERNS | |
| } | |
| videoCallExtraction: { | |
| enabled: true, | |
| patterns: DEFAULT_VIDEO_CALL_PATTERNS.map(pattern => ({ ...pattern })) | |
| } |
🤖 Prompt for AI Agents
In src/settings/ICSSettings.ts around lines 91-94,
DEFAULT_SETTINGS.videoCallExtraction.patterns currently references
DEFAULT_VIDEO_CALL_PATTERNS directly which allows runtime mutations to alter the
exported defaults; change the assignment to deep-clone
DEFAULT_VIDEO_CALL_PATTERNS when building DEFAULT_SETTINGS (use structuredClone
or JSON.parse(JSON.stringify(...))) so the settings get their own copy, and
update the migration code that copies/initializes videoCallExtraction patterns
to also deep-clone each entry rather than copying references so migrations don’t
propagate mutated defaults.
src/settings/ICSSettingsTab.ts
Outdated
| if (!this.plugin.data.videoCallExtraction?.patterns) { | ||
| if (!this.plugin.data.videoCallExtraction) { | ||
| this.plugin.data.videoCallExtraction = { | ||
| enabled: true, | ||
| patterns: DEFAULT_VIDEO_CALL_PATTERNS | ||
| }; | ||
| } else { | ||
| this.plugin.data.videoCallExtraction.patterns = DEFAULT_VIDEO_CALL_PATTERNS; | ||
| } |
There was a problem hiding this comment.
Avoid mutating DEFAULT_VIDEO_CALL_PATTERNS at runtime
DEFAULT_VIDEO_CALL_PATTERNS is being stored by reference in user settings. As soon as a user adds/edits/deletes a pattern, the constant array (and therefore the defaults shipped with the plugin) get mutated, which breaks “Reset to Defaults” and can leak user-specific patterns into future sessions/migrations. Please hydrate videoCallExtraction.patterns with deep copies instead of the shared constant.
- this.plugin.data.videoCallExtraction = {
- enabled: true,
- patterns: DEFAULT_VIDEO_CALL_PATTERNS
- };
+ this.plugin.data.videoCallExtraction = {
+ enabled: true,
+ patterns: DEFAULT_VIDEO_CALL_PATTERNS.map((pattern) => ({ ...pattern })),
+ };
} else {
- this.plugin.data.videoCallExtraction.patterns = DEFAULT_VIDEO_CALL_PATTERNS;
+ this.plugin.data.videoCallExtraction.patterns = DEFAULT_VIDEO_CALL_PATTERNS.map((pattern) => ({ ...pattern }));
}🤖 Prompt for AI Agents
In src/settings/ICSSettingsTab.ts around lines 78–86, the code assigns
DEFAULT_VIDEO_CALL_PATTERNS by reference into
this.plugin.data.videoCallExtraction.patterns, which lets runtime edits mutate
the shared constant; replace those assignments with a deep copy (e.g.,
structuredClone(DEFAULT_VIDEO_CALL_PATTERNS) or
JSON.parse(JSON.stringify(DEFAULT_VIDEO_CALL_PATTERNS)) or lodash/cloneDeep) in
both branches so the settings hold an independent copy and subsequent user edits
do not modify the module-level constant.
Adds configurable field extraction patterns allowing users to extract any type of data from calendar events into custom fields, while maintaining backward compatibility with existing video call URL extraction.
What Changed
callUrlandcallTypefields continue to work unchangedFeatures
Pattern Types
Default Patterns (Backward Compatible)
Field Organization
Settings UI Improvements
Data Access
Legacy API (Backward Compatible)
New Generic API (Recommended)
Use Cases
Technical Details
callUrl/callTypecontinue to workMigration Path
Existing users:
callUrlandcallTypeproperties still populated from "Video Call URLs" fieldextractedFieldsAPI for more flexibilityNew users:
extractedFieldsAPI available from day one🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]
Summary by CodeRabbit
New Features
Documentation
Style