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

V8/bugfix/3498 entity service updates #5658

Merged
merged 3 commits into from
Jun 17, 2019

Conversation

Shazwazza
Copy link
Contributor

@Shazwazza Shazwazza commented Jun 13, 2019

Fixes #3498

Please read notes in the tasks and related links.

The gist is:

  • The EntityService for media was querying all media properties which is slow and shouldn't be done (here's a description of why it was loading in all props: EntityRepository is fetching all property data for media #3460 (comment))
  • It did this in order to get the media's file path, but we can do this a nicer way by joining the umbracoMediaVersion table which contains the media path

So this is all done and tons of code is cleaned up/removed. I'd wager that nobody was ever using the code that was removed. There would be no reason why anyone was using these services and passing in a true value for loadBaseType (or full).

The _mediaFileResolvers stuff that was mentioned in one of those links still remains and that is fine, they are still used to resolve a media's path based on a full media item (i.e. when you click on a media folder in the media section... that said ... it would be much much faster and better performance to load in all media items and thumbnails when you click on a folder to use the EntityService, something to consider in the future since now it uses the MediaService which is loading in all property data!)

Testing

  • Make sure that the media picker still works and shows a thumbnail
  • Make sure that the media grid works and shows thumbnails when you click on a folder in the media tree that contains images
  • Make sure unit tests are all passing
  • review code

@Shazwazza Shazwazza marked this pull request as ready for review June 13, 2019 13:48
@bergmania
Copy link
Member

Tested and code changes looks very good. I'll merge.

We could consider to create constants for some of the magic strings, that are used multiple times "MediaPath", "ContentTypeAlias"

@bergmania bergmania merged commit 9a45bda into v8/dev Jun 17, 2019
@nul800sebastiaan nul800sebastiaan deleted the v8/bugfix/3498-EntityService-updates branch July 3, 2019 21:07
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.

v8 EntityService changes
2 participants