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

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

Merged
merged 3 commits into from
May 9, 2019
Merged

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

merged 3 commits into from
May 9, 2019

Conversation

tribela
Copy link
Contributor

@tribela tribela commented May 8, 2019

Related: #10731

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

Copy link
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
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
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
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
Contributor

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

@tribela
Copy link
Contributor Author

tribela commented May 9, 2019

Thanks!
looks fine now.

Copy link
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.

db/schema.rb Outdated
t.datetime "created_at", default: -> { "now()" }, null: false
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
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
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