Skip to content
This repository has been archived by the owner on Sep 2, 2021. It is now read-only.

Content relations can't be deleted from the "other end" #156

Closed
kraftner opened this issue Aug 27, 2015 · 8 comments
Closed

Content relations can't be deleted from the "other end" #156

kraftner opened this issue Aug 27, 2015 · 8 comments
Assignees
Milestone

Comments

@kraftner
Copy link
Contributor

If you try to remove a content relation from the end it wasn't created originally this doesn't work.

The problem seems to be in Mlp_Content_Relations.php->delete_relation. The where-clause doesn't consider this situation.

I'll continue playing around to see if coming from the wrong side also is an issue with other things like changing a relation.

I also wonder if this is an intended limitation or a known issue.

@kraftner kraftner changed the title content relations aren' Content relations can't be deleted from the "other end" Aug 27, 2015
@tfrommen
Copy link
Member

Closed as related to #143.

@kraftner
Copy link
Contributor Author

I do not understand how that relates to #143.

#143 is about deleting the post. This is about deleting the relation as in "Change Relationship" => "Remove Relationship".

@tfrommen
Copy link
Member

Ah, I see. Sorry for that. This will, however, be resolved as soon as the Content Relations refactor is done. Maybe we can provide a temporary fix for this sometime sooner, though.

@tfrommen
Copy link
Member

Would you mind checking if the according branch works (for you)?

@kraftner
Copy link
Contributor Author

Yes this fixes it for me.

Just out of curiosity - what is your reasoning to fix this like this and not inside Mlp_Content_Relations.php->delete_relation?
As the relations aren't directional (even though they are in the DB) I had assumed that should be standard behaviour that is always handled the same. Or is this something you do not want to touch now as it will be covered by the Content Relations refactor?

@tfrommen
Copy link
Member

Or is this something you do not want to touch now as it will be covered by the Content Relations refactor?

Yes.

@kraftner
Copy link
Contributor Author

I see.

@tfrommen tfrommen self-assigned this Aug 28, 2015
@tfrommen
Copy link
Member

Closed with 9b54f88.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants