Skip to content

Commit d87be23

Browse files
committed
Fix tag processing to not skip tags and in cases where a content item has multiple tag properties
This fixes two issues: - In the ValidateTags method tags were removed from the list of tags (as the copy made in CheckContentProperties was by reference, it would mutate the original list). This would cause subsequent content items to be processed to not include that tag, leading to incorrect results. This is fixed by not modifying the collection at all, but simply computing the set difference between all tags and the tags that the content is tagged with using Except - When a content item had multiple tag properties, the logic would in theory remove all occurences of the content item in tags that were not in the last tag property. By chance this did not happen, because of the above bug. By rewriting the logic to first collect the tags from all properties, this issue is (re)avoided
1 parent 2bcdff8 commit d87be23

File tree

1 file changed

+16
-26
lines changed

1 file changed

+16
-26
lines changed

src/Geta.Optimizely.Tags/TagsScheduledJob.cs

+16-26
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ public override string Execute()
6363

6464
if (content == null || (content is PageData data && data.IsDeleted))
6565
{
66-
RemoveFromAllTags(contentGuid, tags);
66+
RemoveFromTags(contentGuid, tags);
6767
continue;
6868
}
6969

@@ -77,31 +77,23 @@ private void CheckContentProperties(IContent content, IList<Tag> tags)
7777
{
7878
var contentType = _contentTypeRepository.Load(content.ContentTypeID);
7979

80-
foreach (var propertyDefinition in contentType.PropertyDefinitions)
81-
{
82-
if (!TagsHelper.IsTagProperty(propertyDefinition))
83-
{
84-
continue;
85-
}
86-
87-
var tagNames = GetTagNames(content, propertyDefinition);
88-
89-
var allTags = tags;
80+
var tagProperties = contentType.PropertyDefinitions
81+
.Where(TagsHelper.IsTagProperty);
9082

91-
if (tagNames == null)
92-
{
93-
RemoveFromAllTags(content.ContentGuid, allTags);
94-
continue;
95-
}
83+
var contentTagNames = tagProperties
84+
.Select(propertyDefinition => GetTagNames(content, propertyDefinition))
85+
.Where(tagNames => !string.IsNullOrEmpty(tagNames));
9686

97-
var addedTags = ParseTags(tagNames);
87+
var contentTags = contentTagNames
88+
.SelectMany(ParseTags)
89+
.Distinct()
90+
.ToList();
9891

99-
// make sure the tags it has added has the ContentReference
100-
ValidateTags(allTags, content.ContentGuid, addedTags);
92+
// make sure the tags it has added has the ContentReference
93+
ValidateTags(content.ContentGuid, contentTags);
10194

102-
// make sure there's no ContentReference to this ContentReference in the rest of the tags
103-
RemoveFromAllTags(content.ContentGuid, allTags);
104-
}
95+
// make sure there's no ContentReference to this ContentReference in the rest of the tags
96+
RemoveFromTags(content.ContentGuid, tags.Except(contentTags));
10597
}
10698

10799
private string GetTagNames(IContent content, PropertyDefinition propertyDefinition)
@@ -139,12 +131,10 @@ private IEnumerable<Tag> ParseTags(string tagNames)
139131
.ToList();
140132
}
141133

142-
private void ValidateTags(ICollection<Tag> allTags, Guid contentGuid, IEnumerable<Tag> addedTags)
134+
private void ValidateTags(Guid contentGuid, IEnumerable<Tag> addedTags)
143135
{
144136
foreach (var addedTag in addedTags)
145137
{
146-
allTags.Remove(addedTag);
147-
148138
if (addedTag.PermanentLinks.Contains(contentGuid)) continue;
149139

150140
addedTag.PermanentLinks.Add(contentGuid);
@@ -153,7 +143,7 @@ private void ValidateTags(ICollection<Tag> allTags, Guid contentGuid, IEnumerabl
153143
}
154144
}
155145

156-
private void RemoveFromAllTags(Guid guid, IEnumerable<Tag> tags)
146+
private void RemoveFromTags(Guid guid, IEnumerable<Tag> tags)
157147
{
158148
foreach (var tag in tags)
159149
{

0 commit comments

Comments
 (0)