[FIX] Dedup key in Outlook Shared Email Received causing events to be… #18897
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughBumps package and module versions and updates the shared-folder email source to use Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Poller
participant Source as new-email-in-shared-folder
participant OutlookAPI as Microsoft Outlook API
Poller->>Source: trigger poll
Source->>OutlookAPI: listSharedFolderMessages(filter: "sentDateTime gt lastDate", orderBy: "sentDateTime desc")
OutlookAPI-->>Source: items[]
alt items present
loop each item
rect rgb(230,247,255)
Source->>Source: ts = Date.parse(item.sentDateTime)
Source->>Source: eventId = item.id
end
Source-->>Poller: emit { id: eventId, ts, item, subject, ... }
end
else no items
Source-->>Poller: no events
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ 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
🧹 Nitpick comments (1)
components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs (1)
72-72: Consider updating the summary message to useitem.id.The summary message still references
conversationId, which may be confusing since the deduplication key now usesitem.id. For consistency, consider updating the summary to reference the unique message ID:- summary: `A new email with id: "${item.conversationId}" was received!`, + summary: `A new email with id: "${item.id || item.conversationId}" was received!`,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/microsoft_outlook/package.json(1 hunks)components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs(2 hunks)
⏰ 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). (4)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (3)
components/microsoft_outlook/package.json (1)
3-3: LGTM!The version bump from 1.7.3 to 1.7.4 is appropriate for this bug fix.
components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs (2)
9-9: LGTM!The module version bump from 0.0.3 to 0.0.4 aligns with the package version update.
67-67: LGTM!Extracting
createdDateTimeto a local variable improves code clarity and ensures consistent usage.
| item, | ||
| { | ||
| id: item.conversationId, | ||
| id: `${item.id || item.conversationId}-${ts}`, |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that item.id is always present and consider consistency with other sources.
The fix correctly addresses the deduplication issue by using item.id (unique per message) instead of item.conversationId (shared across thread). However, there are a few concerns:
-
Fallback risk: The fallback to
conversationIdwhenitem.idis falsy could reintroduce the original bug for messages without anid. Please verify that the Outlook shared folder messages API always returnsitem.id. -
Timestamp necessity: Appending the timestamp seems unnecessary if
item.idis truly unique per message. According to issue [BUG] Dedup key in Outlook Shared Email Received causing events to be dropped #18884, other Outlook sources useitem.idalone. Consider whether this timestamp appending is needed or if it introduces inconsistency across Outlook sources.
Run the following script to check how other Outlook sources handle deduplication:
Also, please verify the structure of the Outlook shared folder messages API response to confirm item.id is always present:
🏁 Script executed:
#!/bin/bash
# Check deduplication patterns in other Outlook sources
# Find all Outlook source files
fd -e mjs -e js 'sources' components/microsoft_outlook/
# Search for email event emission patterns in Outlook sources
rg -nP -A5 -B5 'this\.\$emit\s*\(' components/microsoft_outlook/sources/ -g '*.mjs' -g '*.js'Length of output: 4667
🌐 Web query:
Does the Microsoft Graph API for listing messages in shared folders always return an id field for each message?
💡 Result:
Short answer: No — it should, but not always in practice.
Details:
- The message resource includes id as a required/read-only property and List messages normally returns an id for each message. [1]
- However, there have been intermittent/known service bugs where Graph responses for mail (and other Outlook/Graph resources) return empty or missing id (or other fields) for some items (reported on StackOverflow / Microsoft forums and Tech Community). If you see it, it’s a server-side issue and not expected behavior of the API. [2][3]
If you want, tell me an example response/request and I can help diagnose (permission, endpoint used for shared mailbox, or whether Prefer: IdType=ImmutableId would help).
Sources:
- Microsoft Graph — message resource (properties include id). [1]
- StackOverflow / developer reports of empty id for Outlook resources. [2]
- Microsoft Tech Community / Q&A reports of missing fields in Graph responses. [3]
🏁 Script executed:
#!/bin/bash
# Check generateMeta implementations in Outlook sources
# Find generateMeta in new-email.mjs
echo "=== new-email.mjs generateMeta ==="
rg -nA10 'generateMeta' components/microsoft_outlook/sources/new-email/new-email.mjs
# Check if there's a base class
echo -e "\n=== Looking for parent classes/mixins ==="
rg -nP 'class|extends|mixins' components/microsoft_outlook/sources/new-email/new-email.mjs | head -5
# Find any base source class
echo -e "\n=== Checking new-email-in-shared-folder parent ==="
rg -nP 'class|extends|mixins' components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs | head -5
# Look for where generateMeta is defined if not in new-email.mjs
echo -e "\n=== Searching for base generateMeta definition ==="
rg -nA5 'generateMeta\s*\(' components/microsoft_outlook/sources/ -g '*.mjs' | head -30Length of output: 3390
🌐 Web query:
Microsoft Graph message id field immutable outlook shared folder API
💡 Result:
Short answer: You can get an immutable message id via Microsoft Graph, but you must opt in (Prefer: IdType="ImmutableId") on every request and access the message in the owning mailbox (shared-folder access uses the owner's mailbox endpoints). Immutable IDs remain stable while the item stays in the same mailbox (moving between folders in that mailbox does NOT change the immutable id), but they will change if the item is moved to an archive mailbox or exported/re‑imported. See Microsoft Graph docs for shared/delegated folder access and for the ImmutableId behavior. [1][2]
Sources:
- Accessing messages in shared/delegated folders (use users/{owner}/mailfolders/... and Mail.Read.Shared / Mail.ReadWrite.Shared permissions). [1]
- Immutable IDs: Prefer: IdType="ImmutableId", lifetime rules, and translateExchangeIds guidance. [2]
Remove timestamp and fallback; align with new-email.mjs pattern using item.id directly.
The verification reveals a critical inconsistency. The related new-email.mjs source handles deduplication using md5(item.id) without a fallback or timestamp. The current fix at line 71 introduces both a fallback and timestamp, which is:
- Inconsistent: No other Outlook email source uses timestamp appending;
new-email.mjsusesmd5(item.id)directly with no fallback - Defeats the fix: The fallback to
conversationIdcould reintroduce the original bug ifitem.idis ever falsy - Unnecessary complexity: Microsoft Graph API message resource includes id as a required/read-only property, so the fallback is defensive for a case that shouldn't occur in normal operation
Recommended fix: Replace line 71 with either id: md5(item.id) (matching new-email.mjs logic) or id: item.id depending on your deduplication requirements. Remove the timestamp—it doesn't provide meaningful deduplication if the primary id field is unreliable, and it breaks consistency with other sources.
🤖 Prompt for AI Agents
In
components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs
around line 71, the id generation currently uses a fallback and timestamp
(`${item.id || item.conversationId}-${ts}`) which is inconsistent with
new-email.mjs and can reintroduce the original bug; remove the timestamp and
fallback and set the id to match the other source (preferably use the same
dedupe strategy: md5(item.id)) so replace the current expression with a direct
md5(item.id) (or item.id if you choose not to hash) and ensure any required md5
import/usage aligns with new-email.mjs.
For Integration QA: |
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check test reports below for more information:
|
jcortes
force-pushed
the
fix-ms-outlook-source-with-wrong-dedupe-key
branch
from
bb688eb to
eecb53d
Compare
|
Hi @vunguyenhung I've just pushed the fix, please try again! thanks! |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs (1)
68-74: Critical: Still usingconversationIdinstead ofitem.idfor deduplication.The current implementation uses
${item.conversationId}-${ts}at line 72, which still relies onconversationId(the thread-level identifier) as the base. This doesn't fully address issue #18884, which explicitly states thatitem.id(the per-message identifier) should be used instead, as done in other Outlook sources.While appending the timestamp makes messages within the same thread unique, this approach:
- Remains inconsistent with other Outlook email sources like
new-email.mjs, which usemd5(item.id)directly- Doesn't follow the recommended pattern from issue #18884
- May contribute to the test failures mentioned by vunguyenhung
Apply this diff to align with the pattern used in
new-email.mjsand fully resolve the issue:- for (const item of responseArray.reverse()) { - const ts = Date.parse(item.createdDateTime); - this.$emit( - item, - { - id: `${item.conversationId}-${ts}`, - summary: `A new email with id: "${item.conversationId}" was received!`, - ts, - }, - ); - } + for (const item of responseArray.reverse()) { + const ts = Date.parse(item.createdDateTime); + this.$emit( + item, + { + id: item.id, + summary: `A new email with id: "${item.id}" was received!`, + ts, + }, + ); + }Note: If you want to hash the id for consistency with
new-email.mjs, import and usemd5frommd5package and setid: md5(item.id).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/microsoft_outlook/package.json(1 hunks)components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/microsoft_outlook/package.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-09T18:07:12.426Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 17538
File: components/aircall/sources/new-sms/new-sms.mjs:19-25
Timestamp: 2025-07-09T18:07:12.426Z
Learning: In Aircall API webhook payloads, the `created_at` field is returned as an ISO 8601 string format (e.g., "2020-02-18T20:52:22.000Z"), not as milliseconds since epoch. For Pipedream components, this needs to be converted to milliseconds using `Date.parse()` before assigning to the `ts` field in `generateMeta()`.
Applied to files:
components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs
⏰ 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). (4)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (1)
components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs (1)
9-9: LGTM: Version bump is appropriate.The version increment follows semantic versioning for a bug fix.
components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs
Outdated
Show resolved
Hide resolved
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check test reports below for more information:
|
jcortes
force-pushed
the
fix-ms-outlook-source-with-wrong-dedupe-key
branch
from
eecb53d to
b6f6b8e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs (1)
73-73: Critical: Deduplication key still usesconversationIdinstead ofitem.id.The current implementation uses
conversationId + timestamp, which only partially addresses issue #18884. This approach has significant problems:
- Root cause not fixed:
conversationIdstill identifies the thread, not individual messages- Timestamp collision risk: Multiple emails in the same thread arriving at the exact same second could still be deduplicated incorrectly
- Inconsistent with other sources: According to the past review comment,
new-email.mjsusesmd5(item.id)for deduplication without fallback or timestamp- Defeats the PR objective: The issue clearly states the fix should use
item.id(per-message identifier) instead ofconversationId(per-thread identifier)Apply this diff to properly fix the deduplication issue:
- id: `${item.conversationId}-${ts}`, + id: item.id,Or to match the pattern in
new-email.mjs, importmd5at the top and use:+import md5 from "md5"; + ... - id: `${item.conversationId}-${ts}`, + id: md5(item.id),Note: This was already flagged in the previous review. The Microsoft Graph API message resource includes
idas a required/read-only property, soitem.idshould always be present. Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/microsoft_outlook/package.json(1 hunks)components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 14265
File: components/the_magic_drip/sources/common.mjs:35-43
Timestamp: 2024-10-10T19:18:27.998Z
Learning: In `components/the_magic_drip/sources/common.mjs`, when processing items in `getAndProcessData`, `savedIds` is intentionally updated with IDs of both emitted and non-emitted items to avoid emitting retroactive events upon first deployment and ensure only new events are emitted as they occur.
📚 Learning: 2025-01-29T22:59:38.825Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 15436
File: components/printful/printful.app.mjs:55-63
Timestamp: 2025-01-29T22:59:38.825Z
Learning: Console.log statements should be removed before merging PRs to maintain code quality and prevent potential security risks from exposing sensitive information in logs.
Applied to files:
components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs
📚 Learning: 2025-02-05T21:58:03.118Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 15479
File: packages/connect-react/src/components/ComponentForm.tsx:23-24
Timestamp: 2025-02-05T21:58:03.118Z
Learning: In the connect-react package, the `enableDebugging` property should be of type `boolean` as it's used for toggling debugging features and conditional rendering.
Applied to files:
components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs
⏰ 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). (4)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (3)
components/microsoft_outlook/package.json (1)
3-3: LGTM - Appropriate version bump for bug fix.The patch version increment follows semantic versioning conventions for a bug fix.
components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs (2)
9-9: LGTM - Module version aligned with package bump.The module version increment is consistent with the package.json version bump.
74-75: Improvement: Summary now includes email subject for better visibility.Including the email subject in the summary provides better context for users monitoring events. Adding
tsto the emitted event is also useful for downstream consumers.However, note that the
tsvalue depends onsentDateTime, which should be verified per the earlier comment.
components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs
Show resolved
Hide resolved
| id: item.conversationId, | ||
| summary: `A new email with id: "${item.conversationId}" was received!`, | ||
| ts: item.createdDateTime, | ||
| id: `${item.conversationId}-${ts}`, |
There was a problem hiding this comment.
why rely on conversation ID + timestamp instead of message ID?
There was a problem hiding this comment.
Hi @alexpareto I've just changed it to item.id
|
Hi @vunguyenhung it seems like you are refering to the conversation/thread in an email message which I don't think would be the right approach to that source component since the main message might have a lot of nested messages. So because the design of the endpoint list only the ones in the shared folder it would not be a good idea to make requests on the each nested message. I hope it makes sense! |
jcortes
force-pushed
the
fix-ms-outlook-source-with-wrong-dedupe-key
branch
2 times, most recently
from
2f970da to
fdcdd7c
Compare
I do think that the event should output a new event when replies are received to the original thread. For example:
That said, with the different item.id I think this should work. |
|
Hi everyone, all test cases are passed! Ready for release! Test reports
|
jcortes
force-pushed
the
fix-ms-outlook-source-with-wrong-dedupe-key
branch
from
fdcdd7c to
b78a8f8
Compare
|
/approve |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs (2)
69-69: Note: Timestamp usessentDateTime.The timestamp computation is correct, but this is another location using
sentDateTimethat should be changed toreceivedDateTime(as flagged in the Critical issue at lines 50-51).
50-51: Critical: Still usingsentDateTimeinstead ofreceivedDateTimefor polling.This issue was flagged in the previous review as Critical. Using
sentDateTimefor polling creates a critical bug: emails sent long ago but received recently will be missed ifsentDateTime < lastDate. Per Microsoft Graph API documentation,receivedDateTimeshould be used for polling "new inbound mail received by the service."Change both lines to use
receivedDateTime:
- Line 50:
$orderBy: "receivedDateTime desc"- Line 51:
$filter: 'receivedDateTime gt ${lastDate}'Also update lines 65 and 69 to use
receivedDateTimefor consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/microsoft_outlook/package.json(1 hunks)components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 14265
File: components/the_magic_drip/sources/common.mjs:35-43
Timestamp: 2024-10-10T19:18:27.998Z
Learning: In `components/the_magic_drip/sources/common.mjs`, when processing items in `getAndProcessData`, `savedIds` is intentionally updated with IDs of both emitted and non-emitted items to avoid emitting retroactive events upon first deployment and ensure only new events are emitted as they occur.
📚 Learning: 2025-01-29T22:59:38.825Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 15436
File: components/printful/printful.app.mjs:55-63
Timestamp: 2025-01-29T22:59:38.825Z
Learning: Console.log statements should be removed before merging PRs to maintain code quality and prevent potential security risks from exposing sensitive information in logs.
Applied to files:
components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs
📚 Learning: 2025-02-05T21:58:03.118Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 15479
File: packages/connect-react/src/components/ComponentForm.tsx:23-24
Timestamp: 2025-02-05T21:58:03.118Z
Learning: In the connect-react package, the `enableDebugging` property should be of type `boolean` as it's used for toggling debugging features and conditional rendering.
Applied to files:
components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs
🔇 Additional comments (5)
components/microsoft_outlook/package.json (1)
3-3: LGTM: Version bump is appropriate.The patch version increment from 1.7.3 to 1.7.4 correctly follows semantic versioning for this bug fix.
components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs (4)
9-9: LGTM: Source version bump is appropriate.The version increment from 0.0.3 to 0.0.4 is consistent with the package-level version bump and appropriate for this bug fix.
64-66: Good: Added defensive check before updating state.The conditional check before calling
_setLastDateprevents updating state when no items are returned, which is good defensive programming.Note: The use of
sentDateTimeon line 65 is part of the Critical issue flagged at lines 50-51.
74-74: LGTM: Summary text improvement.Including the email subject in the summary makes events more user-friendly and easier to identify.
75-75: LGTM: Including timestamp in emission metadata.Adding the
tsfield to the emission metadata is good practice and allows proper event timestamping in the Pipedream platform.Note: The timestamp value is derived from
sentDateTime(line 69), which is part of the Critical issue flagged earlier.
jcortes
force-pushed
the
fix-ms-outlook-source-with-wrong-dedupe-key
branch
from
b78a8f8 to
f1f3bae
Compare
|
/approve |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs (2)
50-51: Critical issue from previous review remains unresolved: UsereceivedDateTimeinstead ofsentDateTime.The previous review identified that using
sentDateTimefor polling creates a critical bug where emails sent long ago but received recently will be missed. Per Microsoft Graph API documentation,receivedDateTime(when Exchange received the message) should be used for polling scenarios, notsentDateTime(when the sender sent the message).This affects ordering (line 50), filtering (line 51), and state persistence (line 65). The bug occurs because
sentDateTimecan be earlier thanreceivedDateTimedue to network delays, offline clients, or delayed delivery.Apply this diff to fix the polling logic:
params: { - $orderBy: "sentDateTime desc", - $filter: `sentDateTime gt ${lastDate}`, + $orderBy: "receivedDateTime desc", + $filter: `receivedDateTime gt ${lastDate}`, },if (responseArray.length) { - this._setLastDate(responseArray[0].sentDateTime); + this._setLastDate(responseArray[0].receivedDateTime); }Also applies to: 64-66
69-69: Timestamp should usereceivedDateTimeto align with polling logic.For consistency with the polling logic (which should use
receivedDateTimeper the previous comment), the event timestamp should also be computed fromreceivedDateTimerather thansentDateTime.Apply this diff:
- const ts = Date.parse(item.sentDateTime); + const ts = Date.parse(item.receivedDateTime);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/microsoft_outlook/package.json(1 hunks)components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-01-29T22:59:38.825Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 15436
File: components/printful/printful.app.mjs:55-63
Timestamp: 2025-01-29T22:59:38.825Z
Learning: Console.log statements should be removed before merging PRs to maintain code quality and prevent potential security risks from exposing sensitive information in logs.
Applied to files:
components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs
📚 Learning: 2025-02-05T21:58:03.118Z
Learnt from: jverce
Repo: PipedreamHQ/pipedream PR: 15479
File: packages/connect-react/src/components/ComponentForm.tsx:23-24
Timestamp: 2025-02-05T21:58:03.118Z
Learning: In the connect-react package, the `enableDebugging` property should be of type `boolean` as it's used for toggling debugging features and conditional rendering.
Applied to files:
components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs
⏰ 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). (4)
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
🔇 Additional comments (3)
components/microsoft_outlook/package.json (1)
3-3: LGTM: Version bump is appropriate.The patch version increment from 1.7.3 to 1.7.4 correctly follows semantic versioning for this bug fix.
components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs (2)
9-9: LGTM: Version bump is appropriate.The version increment from 0.0.3 to 0.0.4 correctly reflects the bug fix changes in this source.
74-74: LGTM: Improved summary text.Using
item.subjectin the summary provides more meaningful information to users compared to the previous id-based summary.
| id: item.conversationId, | ||
| summary: `A new email with id: "${item.conversationId}" was received!`, | ||
| ts: item.createdDateTime, | ||
| id: item.id, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Core deduplication fix is correct.
Using item.id directly (instead of item.conversationId) correctly fixes the bug where replies in the same email thread were being dropped. Each message now has a unique deduplication key, resolving issue #18884.
Optional: Consider md5(item.id) for consistency. The related new-email.mjs source uses md5(item.id) for deduplication. While both approaches work, using md5 here would maintain consistency across Outlook sources.
🤖 Prompt for AI Agents
In
components/microsoft_outlook/sources/new-email-in-shared-folder/new-email-in-shared-folder.mjs
around line 73, the deduplication key currently uses item.id which fixes the bug
but is inconsistent with other Outlook sources that use md5(item.id); update the
dedupe value to use md5(item.id) instead of raw item.id to match new-email.mjs:
import or reference the existing md5 utility, compute md5(item.id) where the id
is assigned, and ensure the rest of the logic uses that hashed string for
deduplication.
… dropped
WHY
Resolves #18884
Summary by CodeRabbit
Bug Fixes
Chores