Skip to content

media tracking references#6936

Merged
Shazwazza merged 13 commits intov8/feature/media-trackingfrom
AB3325-AB3326-AB3327-AB3328-AB3329-AB3330-AB3331-Media-Tracking-References
Oct 31, 2019
Merged

media tracking references#6936
Shazwazza merged 13 commits intov8/feature/media-trackingfrom
AB3325-AB3326-AB3327-AB3328-AB3329-AB3330-AB3331-Media-Tracking-References

Conversation

@bergmania
Copy link
Copy Markdown
Member

@bergmania bergmania commented Oct 29, 2019

Track references of documents and media from

  • Media Picker
  • Content Picker
  • Multi Node Tree Picker
  • Multi Url Picker
  • Grid
  • Nested Content

Fixes AB#3325
Fixes AB#3326
Fixes AB#3327
Fixes AB#3328
Fixes AB#3329
Fixes AB#3331

Tests

  • Adding media and/or content to all of the listed property types and verify the relations shows up under Settings > Relation Types >Related Media > Relations and Settings >Relation Types >Related Document > Relations
  • Also try double nested content.

Copy link
Copy Markdown
Contributor

@Shazwazza Shazwazza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have added some comments to the code, just small things - I'll fix them up now so you don't need to worry about it and then i'll test.

Comment thread src/Umbraco.Web/PropertyEditors/ContentPickerPropertyEditor.cs Outdated
Comment thread src/Umbraco.Web/PropertyEditors/GridPropertyEditor.cs
Comment thread src/Umbraco.Web/PropertyEditors/MediaPickerPropertyEditor.cs Outdated
Comment thread src/Umbraco.Web/PropertyEditors/MultiNodeTreePickerPropertyEditor.cs Outdated
Comment thread src/Umbraco.Web/PropertyEditors/NestedContentPropertyEditor.cs Outdated
{
var propValues = (JObject) o;

var contentType = GetElementType(propValues);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an N+1 call, but I realize that this is being done in the ConvertDbToString method too. I also realize that all content types are cached, so it's not hitting the DB each time but still something to ponder ;)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On that note, this code is basically all duplicated from the ConvertDbToString method. We should prob make a re-usable method to do this iteration

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this was also what I did first and I ended up in something similar to what you did, but I was not proud of the readability of the delegate similar to this: Action<string, PropertyType, JObject, int> onIteration. 😢

But the code duplication was not good either.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's not the prettiest but I'll take it over the duplication, at least its isolated to this editor.

@Shazwazza Shazwazza merged commit ea104e5 into v8/feature/media-tracking Oct 31, 2019
@Shazwazza Shazwazza deleted the AB3325-AB3326-AB3327-AB3328-AB3329-AB3330-AB3331-Media-Tracking-References branch October 31, 2019 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants