Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

U4-6724 Moving content with JSON Tags add extra characters #733

Merged
merged 1 commit into from Jul 2, 2015

Conversation

@base33
Copy link
Contributor

commented Jun 28, 2015

Fixes U4-6724

Problem:
The TagPropertyDefinition class that provides the tag definition on how to parse the tag is always in CSV modes. This affects the way the TagExtractor parses the value - which is causing this issue. It causes it to split by comma instead of treating it as Json.

There was also an additional issue when prevalues were retrieved for the tag property type, line 456 in VersionableRepositoryBase. Distinct was being called, which meant 'storageType' wasn't being returned and only 'group' was being returned. Shortly after this, the TagExtractor is called with the prevalues attached to the ContentPropertyData.

Solution:
I fixed the line retrieving prevalues on line 456 so all prevalues are retrieved.
I have also updated the TagPropertyDefinition to check the 'storageType' prevalue and check if it is set to be JSON and set the TagCacheStorageType accordingly.

@base33

This comment has been minimized.

Copy link
Contributor Author

commented Jun 29, 2015

@Shazwazza @nul800sebastiaan unsure why the AppVeyor build broke? Can you let me know if it is something I need to fix?

@base33

This comment has been minimized.

Copy link
Contributor Author

commented Jun 29, 2015

AppVeyor build is failing due to a test failing. Same test failing in all pull requests.

@@ -454,7 +454,6 @@ INNER JOIN

//this property has tags, so we need to extract them and for that we need the prevals which we've already looked up
var preValData = allPreValues.Value.Where(x => x.DataTypeNodeId == property.PropertyType.DataTypeDefinitionId)
.Distinct()

This comment has been minimized.

Copy link
@Shazwazza

Shazwazza Jul 1, 2015

Member

Not sure if this is the correct fix, if Distinct() isn't actually returning distinct values correctly, the the underyling issue is probably that DataTypePreValueDto hasn't implemented Equals or GetHashCode correctly.
Currently I can see that it is only using 'Id' to do this equals/hash check which might be incorrect.... since as you say it is actually filtering all of the prevalues for this data type.

This comment has been minimized.

Copy link
@base33

base33 Jul 1, 2015

Author Contributor

@Shazwazza makes sense, I'll look into this and do another pull request for the issue.

This comment has been minimized.

Copy link
@Shazwazza

Shazwazza Jul 1, 2015

Member

You should just be able to update the current PR by pushing code to this branch... prob don't need to close and re-create.

This comment has been minimized.

@base33

This comment has been minimized.

Copy link
Contributor Author

commented Jul 1, 2015

@Shazwazza I have now committed the fix. The equals was fine. However, the id was always 0 and was renamed in the sql command which wasn't being used. The issue is now fixed.

Let me know if there are any other changes required

@Shazwazza

This comment has been minimized.

Copy link
Member

commented Jul 2, 2015

Wonderful! ... that may have been causing other errors elsewhere too. I'll pull it and review. Thx!

Shazwazza added a commit that referenced this pull request Jul 2, 2015
U4-6724 Moving content with JSON Tags add extra characters
@Shazwazza Shazwazza merged commit 4257d85 into umbraco:dev-v7 Jul 2, 2015
1 check failed
1 check failed
continuous-integration/appveyor AppVeyor build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.