-
Notifications
You must be signed in to change notification settings - Fork 138
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
Allow for media cover deletion #387
Conversation
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.
sssiiiighqlite 🙀
migrations/sqlite/2018-12-25-164502_media-cover-deletion/down.sql
Outdated
Show resolved
Hide resolved
Maybe this change should be integrated in the migrations that introduced constraints, because when you run migrations on an instance with illustrated articles, you get this error:
(After seeing this error on baptiste.gelez.xyz I tried to manually fix it, but it just ended in an instance reset 😬) (I don't think it is a good idea to change previous migrations, but we don't really have choice…) |
@BaptisteGelez I don't understand, what error did you run into? Is it when an article from before |
It seem to happen when a media is used both as avatar and post cover. This query deletes it: https://github.com/Plume-org/Plume/blob/master/migrations/postgres/2018-12-08-175515_constraints/up.sql#L51 but it fails because it is used on a post. Edit: actually, the media doesn't need to be used as avatar. If it owned by a duplicate user, the query will fail. Edit 2: the user doesn't even need to be duplicated. For instance, some user have an empty |
Codecov Report
@@ Coverage Diff @@
## master #387 +/- ##
==========================================
+ Coverage 27.86% 29.29% +1.42%
==========================================
Files 63 63
Lines 6280 7671 +1391
==========================================
+ Hits 1750 2247 +497
- Misses 4530 5424 +894 |
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.
Travis is happy, I think we can safely merge that. Thanks!
I'm facing the same issue described by @BaptisteGelez while applying this migration, and I cannot afford to reset my instance. Is there anything to do to manually fix the issue? I can run SQL by hand if needed :) |
@EliotBerriot try to find user with empty If it still doesn't work, try to check |
@BaptisteGelez This is the current returns for all the attributes you mentions:
Unfortunately, applying the migrations return the same error again:
The issues seems to lie within the medias/posts relation, so I'm not sure what it has to do with users? Anyway, don't be sorry for this: I know the software was still in development and I could face this kind of bug, that was part of the deal :) |
Okay, so maybe try to reset the post covers (it will not impact your articles as you didn't used them for Funkwhale's blog), with |
@BaptisteGelez at least with the
The issues is withing the |
I made this script that should delete comments with invalid ActivityPub URLs, and their replies. CREATE OR REPLACE FUNCTION delete_comment (id int) RETURNS comments
AS $$
DECLARE
r int;
BEGIN
FOR r IN (SELECT id FROM comments WHERE in_response_to_id = id) LOOP
SELECT delete_comment(r);
END LOOP;
DELETE FROM comments WHERE id = id;
END $$ LANGUAGE plpgsql;
SELECT delete_comment(id) FROM comments WHERE ap_url = ''; I didn't tested it, but it should only delete broken comments (and hopefully fix the migrations). |
@BaptisteGelez unfortunately, this doesn't fix it (for me):
|
Fix #356