-
Notifications
You must be signed in to change notification settings - Fork 1
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
ISOM-1292: Add publish endpoint #381
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
839db7a
to
724aeb1
Compare
7d96701
to
5478fe4
Compare
5478fe4
to
3af1c1b
Compare
3c993ca
to
428ba73
Compare
3af1c1b
to
50d871a
Compare
50d871a
to
ff7ac7d
Compare
ff7ac7d
to
c150004
Compare
{publishButtonProps.showPublish && ( | ||
<PublishButton | ||
pageId={publishButtonProps.pageId} | ||
siteId={publishButtonProps.siteId} | ||
/> | ||
)} |
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.
question(non-blocking): should the button be completely hidden, or should it be shown but hidden?
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.
Completely hiding might confuse the user as to why they can't publish. I prefer to show but disable and add a Tooltip saying:
"Publish will be enabled if there are unpublished edits"
@sehyunidaaa lmk your thoughts!
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.
Yup agree, it should be shown but disabled! As for the copy, maybe All changes have been published
?
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.
some minor comments, happy to approve after!
const addedVersion = await db | ||
.insertInto("Version") | ||
.values({ | ||
versionNum, |
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.
why do callers need to pas in a versionNum
? wouldn't we create the version at 0?
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.
A resource has multiple versions -> every publish on the resource will increment its version by 1
Version numbering starts from 1
.values({ | ||
versionNum, | ||
resourceId: String(resourceId), | ||
blobId: String(blobId), |
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.
not directly actionable but how will we utilise this blobId
? the reason i'm asking is because this is a reference so even if we publish, the value of the blob itself can still change.
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.
Once published the blob shouldn't change, it will be tied closely to the version. on the page resource, draftBlobId is immediately set to null so any future edits will be captured as a new blob
e1d76ed
to
6c858fc
Compare
TL;DR
This pull request introduces a publish endpoint and
PublishButton
component to theAppNavbar
to enable page publishing.What changed?
AppNavbar
to a new folder structure:components/AppNavbar/AppNavbar.tsx
.PublishButton
component toAppNavbar
.PublishButton.tsx
with page and site parameters from the navigation path.publishPage
to handle the publishing logic and update the database accordingly.page.router.ts
andpage.service.ts
with publish logic.Brief description of logic:
Pending Todos:
How to test?
Publish
button.Why make this change?
This feature allows users to publish pages directly from the navigation bar, improving the publishing workflow and user experience.