Skip to content

Conversation

@markmiro
Copy link
Contributor

@markmiro markmiro commented Aug 28, 2025

ai-generated soft delete

"SELECT owner_id, created_at FROM notebooks WHERE id = ? AND deleted_at IS NULL"
)
.bind(notebookId)
.first<{ owner_id: string; created_at: string }>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're listing permissions on this particular notebook, I feel like we don't have to do the delete check. That's metadata to worry about when choosing whether or not to show it in the notebook dashboard.

.prepare("SELECT 1 FROM notebooks WHERE id = ? AND owner_id = ?")
.prepare(
"SELECT 1 FROM notebooks WHERE id = ? AND owner_id = ? AND deleted_at IS NULL"
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for this one. Someone can still own a deleted notebook.


// Delete notebook
// Delete notebook (soft delete)
deleteNotebook: authedProcedure
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could probably call this softDeleteNotebook now, so that someday when we bring in real delete that has a way to go.

WHERE user_id = ? AND permission = 'writer' AND notebook_id IN (${placeholders})`
`SELECT np.notebook_id FROM notebook_permissions np
INNER JOIN notebooks n ON np.notebook_id = n.id
WHERE np.user_id = ? AND np.permission = 'writer' AND n.deleted_at IS NULL AND np.notebook_id IN (${placeholders})`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think the deleted_at check goes outside of the permissions provider.

@markmiro
Copy link
Contributor Author

Closing for now because I haven't touched this in a while

@markmiro markmiro closed this Oct 14, 2025
@rgbkrk rgbkrk deleted the soft-delete branch October 24, 2025 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants