Skip to content

Record deleted(by mod) status to prevent re-appear#10732

Merged
Gargron merged 3 commits intomastodon:masterfrom
tribela:fix-mod-delete
May 9, 2019
Merged

Record deleted(by mod) status to prevent re-appear#10732
Gargron merged 3 commits intomastodon:masterfrom
tribela:fix-mod-delete

Conversation

@tribela
Copy link
Copy Markdown
Contributor

@tribela tribela commented May 8, 2019

Related: #10731

Like Tombstone, deleted status by moderation should not be re-appeared on timeline.

Copy link
Copy Markdown
Contributor

@ClearlyClaire ClearlyClaire left a comment

Choose a reason for hiding this comment

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

db/schema.rb is missing the new table, and app/models/redacted_status.rb is missing the annotations.

Also, I wonder if it wouldn't be better to add a flag in the tombstones table instead, to perform only one lookup instead of two.

@tribela
Copy link
Copy Markdown
Contributor Author

tribela commented May 9, 2019

Thanks for comment. I agree that using tombstone instead of making another table is performance better.
But that status is not actually deleted. So I am curious it is fine to add to Tombstone.

@ClearlyClaire
Copy link
Copy Markdown
Contributor

Yes, I think I'd add a field to Tombstone to mark that it was manually discarded by an admin rather than actually deleted.

@tribela
Copy link
Copy Markdown
Contributor Author

tribela commented May 9, 2019

Oh that's seems good.
Anyway, I am not rails expert. Can you tell me how to add annotation comments?

@ClearlyClaire
Copy link
Copy Markdown
Contributor

They are added when running the migration in the development environment. Usually I add them manually 🤷

@tribela
Copy link
Copy Markdown
Contributor Author

tribela commented May 9, 2019

Thanks!
looks fine now.

Copy link
Copy Markdown
Contributor

@ClearlyClaire ClearlyClaire left a comment

Choose a reason for hiding this comment

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

Also, it looks like the migration script is missing, now.

Comment thread db/schema.rb
t.datetime "updated_at", default: -> { "now()" }, null: false
t.datetime "created_at", default: -> { "CURRENT_TIMESTAMP" }, null: false
t.datetime "updated_at", default: -> { "CURRENT_TIMESTAMP" }, null: false
t.index ["account_id", "status_id"], name: "index_status_pins_on_account_id_and_status_id", unique: true
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not sure why these changes were made?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

IDK too.. But db:migrate makes that.

@Gargron Gargron merged commit ce86356 into mastodon:master May 9, 2019
hiyuki2578 pushed a commit to ProjectMyosotis/mastodon that referenced this pull request Oct 2, 2019
* Record deleted(by mod) status to prevent re-appear

* Move to Tombstone

* Add missing migration script
hiyuki2578 pushed a commit to ProjectMyosotis/mastodon that referenced this pull request Oct 2, 2019
* Record deleted(by mod) status to prevent re-appear

* Move to Tombstone

* Add missing migration script
messenjahofchrist pushed a commit to Origin-Creative/mastodon that referenced this pull request Jul 30, 2021
* Record deleted(by mod) status to prevent re-appear

* Move to Tombstone

* Add missing migration script
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