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

Only get definitions of provided contained types #13891

Merged
merged 3 commits into from Jun 21, 2023
Merged

Conversation

jtkech
Copy link
Member

@jtkech jtkech commented Jun 21, 2023

Fixes #13890

Not really an issue but after seeing the last meeting I took a look at #13768 where in ContainedPartDisplayDriver we changed this.

private IEnumerable<ContentTypeDefinition> GetContainedContentTypes(ListPartSettings settings)
{
  var contentTypes = settings.ContainedContentTypes ?? Enumerable.Empty<string>();

  return contentTypes.Select(contentType => _contentDefinitionManager.GetTypeDefinition(contentType));
}

By that.

private IEnumerable<ContentTypeDefinition> GetContainedContentTypes(ListPartSettings settings)
{
  var contentTypes = settings.ContainedContentTypes ?? Enumerable.Empty<string>();

  if (!contentTypes.Any())
  {
      return Enumerable.Empty<ContentTypeDefinition>();
  }

  return _contentDefinitionManager.ListTypeDefinitions().Where(x => contentTypes.Contains(x.Name));
}
  • Maybe not so important but before we were calling GetTypeDefinition() only for the provided types, but now we call ListTypeDefinitions() that calls GetTypeDefinition() for all existing content types.

  • I also noticed that GetTypeDefinition() is case insentive which is no longer the case with .Contains(x.Name). Note: the var name x of the delegate could also be changed.

  • Reading at Missing content definition in a ListPartSettings throws an exception #13767 it was to fix an NRE when GetTypeDefinition() return null definitions, and that was fixed by retrieving all existing types in place of one by one with possible null definitions.

So here we only get the definitions of the provided types as before, but with a new null check.

Edited: Just tried, if the definition of a contained type is removed it still throws an NRE with a null definition when checking for dynamic content type permissions, even if I keep the existing code, so looks like that #13767 was not fixed, and to be able to edit the item having the list part we need to specify a contained type of an existing type, will see tomorrow.

@jtkech jtkech changed the title Jtkech/contained part Only get definitions of provided contained types Jun 21, 2023
@jtkech jtkech merged commit 76216b9 into main Jun 21, 2023
3 checks passed
@jtkech jtkech deleted the jtkech/contained-part branch June 21, 2023 21:49
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.

Only get definitions of provided contained types
2 participants