Skip to content

Commit 4514244

Browse files
JeremyCaneyJeremyCaney
committed
Merge branch 'bugfix/RelationshipTopicCollection.RemoveTopic-IncomingRelationships' into develop
When adding a relationship via `SetTopic()`, a reciprocal relationship is created on the target (related) topic via its `IncomingRelationships` collection. This wasn't happening when `RemoveTopic()` is called. This could result in stale content and even bugs if a relationship was removed (e.g., via the editor) and the view related to a target (related) topic displayed content from the `IncomingRelationships` collection. To fix this, `RemoveTopic()` now removes the reciprocal relationship in `IncomingRelationships` as well. While I was at it, I also centralized the code between the two `RemoveTopic()` overloads, added additional commenting for readability, and added a couple of unit tests to help validate both the new and existing functionality.
2 parents a2ac659 + 5e1d4d4 commit 4514244

2 files changed

Lines changed: 105 additions & 11 deletions

File tree

‎OnTopic.Tests/RelatedTopicCollectionTest.cs‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,47 @@ public void SetTopic_IsDirty() {
5656
}
5757

5858
/*==========================================================================================================================
59-
| TEST: SET TOPIC: CREATES INCOMING RELATIONSHIP
59+
| TEST: REMOVE TOPIC: REMOVES RELATIONSHIP
60+
\-------------------------------------------------------------------------------------------------------------------------*/
61+
/// <summary>
62+
/// Sets a relationship and then removes it by key, and confirms that it is removed.
63+
/// </summary>
64+
[TestMethod]
65+
public void RemoveTopic_RemovesRelationship() {
66+
67+
var parent = TopicFactory.Create("Parent", "Page");
68+
var related = TopicFactory.Create("Related", "Page");
69+
70+
parent.Relationships.SetTopic("Friends", related);
71+
parent.Relationships.RemoveTopic("Friends", related.Key);
72+
73+
Assert.IsNull(parent.Relationships.GetTopics("Friends").FirstOrDefault());
74+
75+
}
76+
77+
/*==========================================================================================================================
78+
| TEST: REMOVE TOPIC: REMOVES INCOMING RELATIONSHIP
79+
\-------------------------------------------------------------------------------------------------------------------------*/
80+
/// <summary>
81+
/// Sets a relationship and then removes it by key, and confirms that it is removed from the incoming relationships
82+
/// property.
83+
/// </summary>
84+
[TestMethod]
85+
public void RemoveTopic_RemovesIncomingRelationship() {
86+
87+
var parent = TopicFactory.Create("Parent", "Page");
88+
var related = TopicFactory.Create("Related", "Page");
89+
var relationships = new RelatedTopicCollection(parent);
90+
91+
relationships.SetTopic("Friends", related);
92+
relationships.RemoveTopic("Friends", related.Key);
93+
94+
Assert.IsNull(related.IncomingRelationships.GetTopics("Friends").FirstOrDefault());
95+
96+
}
97+
98+
/*==========================================================================================================================
99+
| TEST: REMOVE TOPIC: REMOVES INCOMING RELATIONSHIP
60100
\-------------------------------------------------------------------------------------------------------------------------*/
61101
/// <summary>
62102
/// Sets a relationship and confirms that it is accessible on incoming relationships property.

‎OnTopic/Collections/RelatedTopicCollection.cs‎

Lines changed: 64 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -131,37 +131,91 @@ public void ClearTopics(string relationshipKey) {
131131
/// </summary>
132132
/// <param name="relationshipKey">The key of the relationship.</param>
133133
/// <param name="topicKey">The key of the topic to be removed.</param>
134+
/// <param name="isIncoming">
135+
/// Notes that this is setting an internal relationship, and thus shouldn't set the reciprocal relationship.
136+
/// </param>
134137
/// <returns>
135138
/// Returns true if the <see cref="Topic"/> is removed; returns false if either the relationship key or the
136139
/// <see cref="Topic"/> cannot be found.
137140
/// </returns>
138-
public bool RemoveTopic(string relationshipKey, string topicKey) {
141+
public bool RemoveTopic(string relationshipKey, string topicKey, bool isIncoming = false) {
142+
143+
/*------------------------------------------------------------------------------------------------------------------------
144+
| Validate contracts
145+
\-----------------------------------------------------------------------------------------------------------------------*/
139146
Contract.Requires<ArgumentNullException>(!String.IsNullOrWhiteSpace(relationshipKey));
140147
Contract.Requires<ArgumentNullException>(!String.IsNullOrWhiteSpace(topicKey));
141-
if (Contains(relationshipKey)) {
142-
var topics = this[relationshipKey];
143-
return topics.Remove(topicKey);
148+
149+
/*------------------------------------------------------------------------------------------------------------------------
150+
| Validate topic key
151+
\-----------------------------------------------------------------------------------------------------------------------*/
152+
var topics = Contains(relationshipKey)? this[relationshipKey] : null;
153+
var topic = topics?.Contains(topicKey)?? false? topics[topicKey] : null;
154+
155+
if (topics == null || topic == null) {
156+
return false;
144157
}
145-
return false;
158+
159+
/*------------------------------------------------------------------------------------------------------------------------
160+
| Call overload
161+
\-----------------------------------------------------------------------------------------------------------------------*/
162+
return RemoveTopic(relationshipKey, topic, isIncoming);
163+
146164
}
147165

148166
/// <summary>
149167
/// Removes a specific <see cref="Topic"/> object associated with a specific relationship key.
150168
/// </summary>
151169
/// <param name="relationshipKey">The key of the relationship.</param>
152170
/// <param name="topic">The topic to be removed.</param>
171+
/// <param name="isIncoming">
172+
/// Notes that this is setting an internal relationship, and thus shouldn't set the reciprocal relationship.
173+
/// </param>
153174
/// <returns>
154175
/// Returns true if the <see cref="Topic"/> is removed; returns false if either the relationship key or the
155176
/// <see cref="Topic"/> cannot be found.
156177
/// </returns>
157-
public bool RemoveTopic(string relationshipKey, Topic topic) {
178+
public bool RemoveTopic(string relationshipKey, Topic topic, bool isIncoming = false) {
179+
180+
/*------------------------------------------------------------------------------------------------------------------------
181+
| Validate contracts
182+
\-----------------------------------------------------------------------------------------------------------------------*/
158183
Contract.Requires<ArgumentNullException>(!String.IsNullOrWhiteSpace(relationshipKey));
159184
Contract.Requires(topic);
160-
if (Contains(relationshipKey)) {
161-
var topics = this[relationshipKey];
162-
return topics.Remove(topic);
185+
186+
/*------------------------------------------------------------------------------------------------------------------------
187+
| Remove reciprocal relationship, if appropriate
188+
\-----------------------------------------------------------------------------------------------------------------------*/
189+
if (!isIncoming) {
190+
if (_isIncoming) {
191+
throw new ArgumentException(
192+
"You are attempting to remove an incoming relationship on a RelatedTopicCollection that is not flagged as " +
193+
"IsIncoming",
194+
nameof(isIncoming)
195+
);
196+
}
197+
topic.IncomingRelationships.RemoveTopic(relationshipKey, _parent, true);
198+
}
199+
200+
/*------------------------------------------------------------------------------------------------------------------------
201+
| Validate relationshipKey
202+
\-----------------------------------------------------------------------------------------------------------------------*/
203+
var topics = Contains(relationshipKey)? this[relationshipKey] : null;
204+
205+
if (topics == null || !topics.Contains(topic)) {
206+
return false;
163207
}
164-
return false;
208+
209+
/*------------------------------------------------------------------------------------------------------------------------
210+
| Remove relationship
211+
\-----------------------------------------------------------------------------------------------------------------------*/
212+
topics.Remove(topic);
213+
214+
/*------------------------------------------------------------------------------------------------------------------------
215+
| Remove true
216+
\-----------------------------------------------------------------------------------------------------------------------*/
217+
return true;
218+
165219
}
166220

167221
/*==========================================================================================================================

0 commit comments

Comments
 (0)