-
Notifications
You must be signed in to change notification settings - Fork 60
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
Merged commits management #44
Conversation
532a603
to
4ea0e26
Compare
return ( | ||
<main className='flex flex-row w-full'> | ||
<div className='w-[280px]'> | ||
<Sidebar /> | ||
</div> | ||
<div className='flex-1'>{children}</div> | ||
</main> | ||
) | ||
return children | ||
} |
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.
Sidebar has been moved to /projects/[projectId]/commits/[commitUuid]/
const CommitContext = createContext<CommitContextType>({ | ||
commitUuid: HEAD_COMMIT, | ||
isDraft: false, | ||
}) |
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.
useCurrentCommit
will return both the commitUuid
and an isDraft
property, which can be used to allow or forbid modifications depending on the commit.
4ea0e26
to
a304c84
Compare
if (commit.mergedAt !== null) { | ||
return Result.error( | ||
new ForbiddenError('Cannot create a document version in a merged commit'), | ||
) | ||
} |
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.
Added this restriction on createDocumentVersion
.
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.
hmm would be cool to have model level validations like activerecord, @andresgutgon do u know if there's something for this in drizzle land?
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 saw this kind of repository pattern example:
https://gist.github.com/cayter/49d5c256a885d90c399ca6c1eca19f51
We could implement it and hook into beforeCreate
to validate the row
https://gist.github.com/cayter/49d5c256a885d90c399ca6c1eca19f51#file-repository-ts-L357
But I didn't see anything official.
I'm a fan of ActiveRecord but Drizzle is super functional. I don't think it is wrong to have a service to create documents with the validation inside. As long as we use it as unique way for creating documents
a304c84
to
37e1555
Compare
37e1555
to
c9f5f38
Compare
mergedAt
property to commitsProjectProvider
andCommitProvider
componentsgetDocumentsAtCommit
and other methods to new schema.Note: Many functions require both
commitUuid
andprojectId
, since thecommitUuid
can be 'HEAD', and we will require the project to know which commit does that represent.