Skip to content
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

Perpetual Edit #1915

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
Draft

Perpetual Edit #1915

wants to merge 13 commits into from

Conversation

Soxasora
Copy link
Member

@Soxasora Soxasora commented Feb 18, 2025

Description

Completes #1049
Enables perpetual editing with history accessible with oldVersions (references OldItem) on Item.

What we want is to guarantee an Item's immutability by storing the original Item before editing. Allowing us to see and compare the editing history.

But, before immutability and history making, we have shadow editing: a 10 minutes period of grace where the user can edit however and how much they want without ever creating a record of it, using the same mechanism every stacker is already used to.

Screenshots

Early UI

Item info, history:
image

Old Item:
image

Old Item with image:
image

Additional Context

The goal was to achieve this via Postgres' polymorphism but Prisma doesn't support inheritance and at the moment we store what really can be changed via editing, which is the content (title, text, url).

We can pivot from Shadow Editing by removing references to it.

We don't record to OldItem if it's a biography (shadow editing).

Design goal

Checklist

Are your changes backwards compatible? Please answer below:
Yes

On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:

7.5: more details

OK comments
OK items that belongs to a territory
OK pinned items that belongs to a territory
OK only the author can edit
OK shadow editing
OK item history
OK old item readability, markdown and images
OK record only after 10 minutes and if content has changed
OK don't record history on bios, jobs, items

For frontend changes: Tested on mobile, light and dark mode? Please answer below:
Yes!

Did you introduce any new environment variables? If so, call them out explicitly here:
No

Progress

  • Fetch old version when selected
  • Add comments
  • Store full Item -- ?
  • Store original Item in OldItem
  • Editing history UI
  • Better trigger function OR pivot to JS handling
  • Item deletion should delete text also in OldItem
  • Edited Item UI
  • Allow edits after 10 minutes

@Soxasora Soxasora changed the title wip Store Item before editing in OldItem Perpetual Edit Feb 18, 2025
@Soxasora Soxasora added the feature new product features that weren't there before label Feb 18, 2025
@Soxasora Soxasora force-pushed the perpetual_edit branch 3 times, most recently from 3fa0464 to 5544450 Compare February 18, 2025 12:37
Comment on lines 68 to 89
// history tracking
// has to be older than 10 minutes, not a bio, not a job, not deleted, not a special item.
const adminItem = ADMIN_ITEMS.includes(old.id)
if (old.createdAt < new Date(Date.now() - 10 * 60 * 1000) && !old.bio && old.subName !== 'jobs' && !data.deletedAt && !adminItem) {
await tx.oldItem.create({
data: {
title: old.title,
text: old.text,
url: old.url,
userId: old.userId,
subName: old.subName,
imgproxyUrls: old.imgproxyUrls,
cloneBornAt: old.cloneBornAt,
cloneDiedAt: new Date(),
originalItemId: parseInt(id)
}
})

data.cloneBornAt = new Date() // we can use this to determine if the item has been edited
data.cloneDiedAt = null
}

Copy link
Member Author

@Soxasora Soxasora Feb 21, 2025

Choose a reason for hiding this comment

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

Since we really just want to track Item content editing, I switched to a Prisma-based version of the trigger that I think it's more appropriate. Before, every update to Item could fire the procedure and confront the conditions.

Also on this, not having Item inheritance made me think that having just enough columns can be okay, but also having the whole item is okay to me, maybe future-proof

@Soxasora Soxasora marked this pull request as ready for review February 21, 2025 20:09
@huumn
Copy link
Member

huumn commented Mar 1, 2025

The non-trigger-based versioning, and of only select fields, makes me a little nervous tbh. I haven't looked at it very carefully at it though. I'll try to take a closer look when I'm less tired.

@Soxasora
Copy link
Member Author

Soxasora commented Mar 1, 2025

Reverting to trigger is something I was debating myself some days ago, I just want to do it better than what it was before switching to Prisma.

I think we can do a 1:1 copy of Item for OldItem! I was selecting just the bare minimum for it to be displayed.

Thanks k00b, redrafting! ^^

@Soxasora Soxasora marked this pull request as draft March 1, 2025 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature new product features that weren't there before
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants