-
Notifications
You must be signed in to change notification settings - Fork 279
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: blobs transcript date filter #3877
Conversation
@@ -141,8 +141,7 @@ export class BlobsTranscriptStore implements TranscriptStore { | |||
const fromIdx = | |||
startDate != null | |||
? blobItems.findIndex( | |||
(blobItem) => | |||
blobItem?.metadata?.timestamp && new Date(blobItem.metadata.timestamp) >= startDate | |||
(blobItem) => blobItem?.properties?.createdOn && blobItem?.properties?.createdOn >= startDate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I logged the blobItems
array and found that metadata
is undefined by default, so I opted for this property that does exist. I tested against a real blob storage container.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is createdOn
already a Date()
obj?
This change still works with local storage emulation, too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is, yes. I'll test with the emulator as well. It should as the Typescript type definitions should be correct. Will double check though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mdrichardson Emulator checks out!
@@ -5,7 +5,7 @@ import * as z from 'zod'; | |||
import getStream from 'get-stream'; | |||
import pmap from 'p-map'; | |||
import { Activity, PagedResult, TranscriptInfo, TranscriptStore } from 'botbuilder-core'; | |||
import { maybeCast } from 'botbuilder-stdlib/lib/maybeCast'; | |||
import { maybeCast } from 'botbuilder-stdlib'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simple fix here, auto-import included the wrong path before.
Pull Request Test Coverage Report for Build 1054042577
💛 - Coveralls |
Fixes #3864