-
Notifications
You must be signed in to change notification settings - Fork 432
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(core): version document's history was returning error #7438
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
No changes to documentation |
Component Testing Report Updated Aug 30, 2024 2:54 PM (UTC) ✅ All Tests Passed -- expand for details
|
Also, not sure what but @jordanl17 seems that the |
@@ -147,7 +149,7 @@ export class Timeline { | |||
id: event.id, | |||
author: event.author, | |||
timestamp: event.timestamp, | |||
draftEffect: event.effects[this.draftId], | |||
draftEffect: event.effects[this.versionId], |
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.
did you consider renaming draftEffect
to versionEffect
?
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 briefly did, but worried it would have too big a footprint. However just looked at it again and draftEffect
's use is quite limited, so I've made this improvement. Thanks
…ic use with versions
3eb6403
to
a5fcbb1
Compare
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.
Seems to work nicely for me. Nice fix! 🥳
* fix: draftId correct set in timeline for version docs * refactor: timeline uses documentId rather than publishedId for agnostic use with versions * refactor(core): defining versionId rather than draftId * refactor(core): refactor to better reflect use of version nomenclature in history * fix(core): resolving tsdoc issue with destructure * fix(core): resolving tsdoc issue with destructure
* fix: draftId correct set in timeline for version docs * refactor: timeline uses documentId rather than publishedId for agnostic use with versions * refactor(core): defining versionId rather than draftId * refactor(core): refactor to better reflect use of version nomenclature in history * fix(core): resolving tsdoc issue with destructure * fix(core): resolving tsdoc issue with destructure
* fix: draftId correct set in timeline for version docs * refactor: timeline uses documentId rather than publishedId for agnostic use with versions * refactor(core): defining versionId rather than draftId * refactor(core): refactor to better reflect use of version nomenclature in history * fix(core): resolving tsdoc issue with destructure * fix(core): resolving tsdoc issue with destructure
* fix: draftId correct set in timeline for version docs * refactor: timeline uses documentId rather than publishedId for agnostic use with versions * refactor(core): defining versionId rather than draftId * refactor(core): refactor to better reflect use of version nomenclature in history * fix(core): resolving tsdoc issue with destructure * fix(core): resolving tsdoc issue with destructure
Description
https://www.loom.com/share/b1e04c690f7b45dcb40d153cf055c97b?sid=6a600c40-cf41-4bc3-ad0c-d0b90bba4414
What to review
publishedId
todocumentId
, is to give affordance that in cases where the document is a version, then publishedId is a misnomer. It was really this line:this.draftId = isVersionId(documentId) ? documentId :
drafts.${documentId}`` where it felt incorrect to refer topublishedId
Timeline
to useversionId
to reference a version Id and a canonical draftTesting
N/A
Notes for release
N/A