#899 frontend: un-pin projects when deleting them
Merged 3 months ago by praiskup. Opened 4 months ago by frostyx.
copr/ frostyx/copr pinned-projects-deletion  into  master

@@ -16,7 +16,14 @@ 

  

  from coprs.logic.users_logic import UsersLogic

  from coprs.models import User, Copr

- from .coprs_logic import CoprsLogic, CoprDirsLogic, CoprChrootsLogic

+ from .coprs_logic import CoprsLogic, CoprDirsLogic, CoprChrootsLogic, PinnedCoprsLogic

+ 

+ 

+ @sqlalchemy.event.listens_for(models.Copr.deleted, "set")

+ def unpin_projects_on_delete(copr, deleted, oldvalue, event):

+     if not deleted:

+         return

+     PinnedCoprsLogic.delete_by_copr(copr)

  

  

  class ComplexLogic(object):

@@ -924,3 +924,9 @@ 

          if isinstance(owner, models.Group):

              return query.filter(models.PinnedCoprs.group_id == owner.id).delete()

          return query.filter(models.PinnedCoprs.user_id == owner.id).delete()

+ 

+     @classmethod

+     def delete_by_copr(cls, copr):

+         return (db.session.query(models.PinnedCoprs)

+                 .filter(models.PinnedCoprs.copr_id == copr.id)

+                 .delete())

don't you want to add on delete cascade?

I thought that on delete cascade works like - I have a table foo which references a table bar through bar_id. Then I set on delete cascade for foo table and remove a row from foo and because of it, it automatically deletes related rows from bar.

However, this is a different case, the relation is the other way. Now we have a table foo which is references from bar through foo_id. And we are deleting a row from table foo (and want to automatically remove rows from bar). Will this work with cascade? I sincerely don't know, I've never set a cascade this way.

Yeah, the on delete cascade would have to be on PinnedCoprs table? In particular, we want to remove PinnedCopr entry if Copr entry is removed.

Ok, the problem is that we don't actually delete the field, we only set deleted=True :-(

Don't you want to try the listen() method then?

Yeah, let's try listen() :-)

@@ -12,6 +12,7 @@ 

  from coprs.logic.actions_logic import ActionsLogic

  from coprs.logic.coprs_logic import CoprsLogic, CoprChrootsLogic, PinnedCoprsLogic

  from coprs.logic.users_logic import UsersLogic

+ from coprs.logic.complex_logic import ComplexLogic

  

  from coprs import models

  from coprs.whoosheers import CoprWhoosheer

@@ -167,3 +168,12 @@ 

              form.copr_ids.data = ["1", "1"]

              assert not form.validate()

              assert "only once" in form.errors["coprs"][0]

+ 

+     def test_delete_project_that_is_pinned(self, f_users, f_coprs, f_db):

+         pc1 = models.PinnedCoprs(id=1, copr_id=self.c2.id, user_id=self.u2.id, position=1)

+         pc2 = models.PinnedCoprs(id=2, copr_id=self.c3.id, user_id=self.u2.id, position=2)

+         self.db.session.add_all([pc1, pc2])

+ 

+         ComplexLogic.delete_copr(self.c2, admin_action=True)

+         assert set(CoprsLogic.get_multiple_by_username(self.u2.name)) == {self.c3}

+         assert set(PinnedCoprsLogic.get_by_owner(self.u2)) == {pc2}

rebased onto f31a760ad53581cd86aa2be6272497948aa67dc9

4 months ago

I thought that on delete cascade works like - I have a table foo which references a table bar through bar_id. Then I set on delete cascade for foo table and remove a row from foo and because of it, it automatically deletes related rows from bar.

However, this is a different case, the relation is the other way. Now we have a table foo which is references from bar through foo_id. And we are deleting a row from table foo (and want to automatically remove rows from bar). Will this work with cascade? I sincerely don't know, I've never set a cascade this way.

Yeah, the on delete cascade would have to be on PinnedCoprs table? In particular, we want to remove PinnedCopr entry if Copr entry is removed.

Ok, the problem is that we don't actually delete the field, we only set deleted=True :-(

Don't you want to try the listen() method then?

1 new commit added

  • frontend: un-pin project through sqlalchemy event listenner
3 months ago

Yeah, let's try listen() :-)

PTAL

Even though I see no benefit in two-patch solution.

rebased onto d7be468

3 months ago

Pull-Request has been merged by praiskup

3 months ago