Allow for media cover deletion #387

병합
Plume_migration_agent delete-cover 에서 master 로 2 commits 를 머지했습니다 5 년 전
trinity-1686a 코멘트됨, 5 년 전
소유자

Fix #356

Fix #356
igalic (Migrated from github.com) 검토됨 5 년 전
igalic (Migrated from github.com) left a comment

sssiiiighqlite 🙀

_sssiiiighqlite_ 🙀
igalic (Migrated from github.com) 코멘트됨, 5 년 전

unfortunately, you cannot drop or add constraints on sqlite tables without doing the create temporary table dance

unfortunately, you cannot drop or add constraints on sqlite tables without doing the create temporary table dance
trinity-1686a 검토됨 5 년 전
trinity-1686a 코멘트됨, 5 년 전
포스터
소유자

Travis made me notice it. I knew for columns, I through it worked at least for constraints (it's only drop, add work just fine if I remember well)

Travis made me notice it. I knew for columns, I through it worked at least for constraints (it's only drop, add work just fine if I remember well)
elegaanz 코멘트됨, 5 년 전 (Migrated from github.com)

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:

Running migration 2018-12-08-175515_constraints
update or delete on table "medias" violates foreign key constraint "posts_cover_id_fkey" on table "posts"

(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…)

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: ``` Running migration 2018-12-08-175515_constraints update or delete on table "medias" violates foreign key constraint "posts_cover_id_fkey" on table "posts" ``` (After seeing this error on baptiste.gelez.xyz I tried to manually fix it, but it just ended in an instance reset :grimacing:) (I don't think it is a good idea to change previous migrations, but we don't really have choice…)
trinity-1686a 코멘트됨, 5 년 전
포스터
소유자

@BaptisteGelez I don't understand, what error did you run into? Is it when an article from before 2018-12-08-175515_constraints reference a no longer existing media?

@BaptisteGelez I don't understand, what error did you run into? Is it when an article from before `2018-12-08-175515_constraints` reference a no longer existing media?
elegaanz 코멘트됨, 5 년 전 (Migrated from github.com)

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 followers_endpoint property, because they were created at a time this property didn't exist.

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 `followers_endpoint` property, because they were created at a time this property didn't exist.
codecov[bot] 코멘트됨, 5 년 전 (Migrated from github.com)

Codecov Report

Merging #387 into master will increase coverage by 1.42%.
The diff coverage is n/a.

@@            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
# [Codecov](https://codecov.io/gh/Plume-org/Plume/pull/387?src=pr&el=h1) Report > Merging [#387](https://codecov.io/gh/Plume-org/Plume/pull/387?src=pr&el=desc) into [master](https://codecov.io/gh/Plume-org/Plume/commit/9b3b79ef9c7a77a60c62c8c3682ed86723e611ba?src=pr&el=desc) will **increase** coverage by `1.42%`. > The diff coverage is `n/a`. ```diff @@ 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 ```
elegaanz (Migrated from github.com) 이 변경사항을 승인하였습니다. 5 년 전
elegaanz (Migrated from github.com) left a comment

Travis is happy, I think we can safely merge that. Thanks!

Travis is happy, I think we can safely merge that. Thanks!
EliotBerriot 코멘트됨, 5 년 전 (Migrated from github.com)

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 :)

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 :)
elegaanz 코멘트됨, 5 년 전 (Migrated from github.com)

@EliotBerriot try to find user with empty followers_endpoint with SELECT id, username FROM users WHERE followers_endpoint = ''; and if anything is returned, edit the follower endpoint so that it has a value (it can be anything as long as it is not null, nor already used by another user). If I remember correctly, that's how I fixed it on fediverse.blog. Sorry for the bug… 😕

If it still doesn't work, try to check users.ap_url, user.inbox_url and user.outbox_url too.

@EliotBerriot try to find user with empty `followers_endpoint` with `SELECT id, username FROM users WHERE followers_endpoint = '';` and if anything is returned, edit the follower endpoint so that it has a value (it can be anything as long as it is not null, nor already used by another user). If I remember correctly, that's how I fixed it on fediverse.blog. Sorry for the bug… :confused: If it still doesn't work, try to check `users.ap_url`, `user.inbox_url` and `user.outbox_url` too.
EliotBerriot 코멘트됨, 5 년 전 (Migrated from github.com)

@BaptisteGelez SELECT id, username FROM users WHERE followers_endpoint = ''; returned a bunch of users, and I updated it w ith UPDATE users SET followers_endpoint='noop' WHERE followers_endpoint = '';`

This is the current returns for all the attributes you mentions:

plume=# SELECT id, username FROM users WHERE ap_url = '';
 id | username 
----+----------
(0 rows)

plume=# SELECT id, username FROM users WHERE inbox_url = '';
 id | username 
----+----------
(0 rows)

plume=# SELECT id, username FROM users WHERE outbox_url = '';
 id | username 
----+----------
(0 rows)

plume=# SELECT id, username FROM users WHERE followers_endpoint = '';
 id | username 
----+----------
(0 rows)

Unfortunately, applying the migrations return the same error again:

eliotberriot@peeters:/srv/Plume$ docker-compose run --rm plume diesel migration run --migration-dir migrations/postgres/
Starting plume_postgres_1 ... done
Running migration 2018-12-08-175515_constraints
update or delete on table "medias" violates foreign key constraint "posts_cover_id_fkey" on table "posts"

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 :)

@BaptisteGelez `SELECT id, username FROM users WHERE followers_endpoint = ''; returned a bunch of users, and I updated it w ith `UPDATE users SET followers_endpoint='noop' WHERE followers_endpoint = '';` This is the current returns for all the attributes you mentions: ``` plume=# SELECT id, username FROM users WHERE ap_url = ''; id | username ----+---------- (0 rows) plume=# SELECT id, username FROM users WHERE inbox_url = ''; id | username ----+---------- (0 rows) plume=# SELECT id, username FROM users WHERE outbox_url = ''; id | username ----+---------- (0 rows) plume=# SELECT id, username FROM users WHERE followers_endpoint = ''; id | username ----+---------- (0 rows) ``` Unfortunately, applying the migrations return the same error again: ``` eliotberriot@peeters:/srv/Plume$ docker-compose run --rm plume diesel migration run --migration-dir migrations/postgres/ Starting plume_postgres_1 ... done Running migration 2018-12-08-175515_constraints update or delete on table "medias" violates foreign key constraint "posts_cover_id_fkey" on table "posts" ``` 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 :)
elegaanz 코멘트됨, 5 년 전 (Migrated from github.com)

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 UPDATE posts SET cover_id = null;. I don't know if it will fix the issue, but it is worth a try.

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 `UPDATE posts SET cover_id = null;`. I don't know if it will fix the issue, but it is worth a try.
EliotBerriot 코멘트됨, 5 년 전 (Migrated from github.com)

@BaptisteGelez at least with the cover_id thing I get another error:

eliotberriot@peeters:/srv/Plume$ docker-compose run --rm plume diesel migration run --migration-dir migrations/postgres/
Starting plume_postgres_1 ... done
Running migration 2018-12-08-175515_constraints
update or delete on table "comments" violates foreign key constraint "comments_in_response_to_id_fkey" on table "comments"

The issues is withing the comments_in_response_to_id_fkey constraint now

@BaptisteGelez at least with the `cover_id` thing I get another error: ``` eliotberriot@peeters:/srv/Plume$ docker-compose run --rm plume diesel migration run --migration-dir migrations/postgres/ Starting plume_postgres_1 ... done Running migration 2018-12-08-175515_constraints update or delete on table "comments" violates foreign key constraint "comments_in_response_to_id_fkey" on table "comments" ``` The issues is withing the `comments_in_response_to_id_fkey` constraint now
elegaanz 코멘트됨, 5 년 전 (Migrated from github.com)

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).

I made this script that should delete comments with invalid ActivityPub URLs, and their replies. ```sql 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).
EliotBerriot 코멘트됨, 5 년 전 (Migrated from github.com)

@BaptisteGelez unfortunately, this doesn't fix it (for me):

eliotberriot@peeters:/srv/Plume$ docker-compose exec postgres psql -U plume
psql (10.5 (Debian 10.5-2.pgdg90+1))
Type "help" for help.

plume=# 
plume=# 
plume=# CREATE OR REPLACE FUNCTION delete_comment (id int) RETURNS comments
plume-# AS $$
plume$# DECLARE
plume$#   r int;
plume$# BEGIN
plume$#   FOR r IN (SELECT id FROM comments WHERE in_response_to_id = id) LOOP
plume$#     SELECT delete_comment(r);
plume$#   END LOOP;
plume$#   DELETE FROM comments WHERE id = id;
plume$# END $$ LANGUAGE plpgsql;
CREATE FUNCTION
plume=# 
plume=# SELECT delete_comment(id) FROM comments WHERE ap_url = '';
 delete_comment 
----------------
(0 rows)

plume=# \q

eliotberriot@peeters:/srv/Plume$ docker-compose run --rm plume diesel migration run --migration-dir migrations/postgres/
Starting plume_postgres_1 ... done
Running migration 2018-12-08-175515_constraints
update or delete on table "comments" violates foreign key constraint "comments_in_response_to_id_fkey" on table "comments"
@BaptisteGelez unfortunately, this doesn't fix it (for me): ``` eliotberriot@peeters:/srv/Plume$ docker-compose exec postgres psql -U plume psql (10.5 (Debian 10.5-2.pgdg90+1)) Type "help" for help. plume=# plume=# plume=# CREATE OR REPLACE FUNCTION delete_comment (id int) RETURNS comments plume-# AS $$ plume$# DECLARE plume$# r int; plume$# BEGIN plume$# FOR r IN (SELECT id FROM comments WHERE in_response_to_id = id) LOOP plume$# SELECT delete_comment(r); plume$# END LOOP; plume$# DELETE FROM comments WHERE id = id; plume$# END $$ LANGUAGE plpgsql; CREATE FUNCTION plume=# plume=# SELECT delete_comment(id) FROM comments WHERE ap_url = ''; delete_comment ---------------- (0 rows) plume=# \q eliotberriot@peeters:/srv/Plume$ docker-compose run --rm plume diesel migration run --migration-dir migrations/postgres/ Starting plume_postgres_1 ... done Running migration 2018-12-08-175515_constraints update or delete on table "comments" violates foreign key constraint "comments_in_response_to_id_fkey" on table "comments" ```

리뷰어

The pull request has been merged as aa72334dc6.
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b delete-cover master
git pull origin delete-cover

Step 2:

Merge the changes and update on Forgejo.
git checkout master
git merge --no-ff delete-cover
git push origin master
로그인하여 이 대화에 참여
No reviewers
마일스톤 없음
담당자 없음
참여자 2명
알림
마감일
기한이 올바르지 않거나 범위를 벗어났습니다. 'yyyy-mm-dd'형식을 사용해주십시오.

마감일이 설정되지 않았습니다.

의존성

No dependencies set.

Reference: Plume/Plume#387
불러오는 중...
아직 콘텐츠가 없습니다.