Beyond Retail - updates for Blog migration to v11 + bugfixes + reimplementing missing bits from v7 package #30
Conversation
…ly with split path, handle non-user author types
… only) + ensure tags are searched using lowercase
MDuncan98
requested review from
stevetemple,
tristanjthompson,
aheaford and
ThomasAdams2
| { | ||
| values.Add(ExamineFields.Tag, tags.Select(t => t.ToLower())); | ||
| // Tags are likely csv rather than JSON |
There was a problem hiding this comment.
Do we want to keep this comment here?
There was a problem hiding this comment.
Good question. I think it explains what's going on cuz it might seem a bit weird that we try to deserialize this value, then do a string.Split() - makes it known this is intentional.
Don't feel strongly about it either way though
tristanjthompson
changed the title
| else if (UdiParser.TryParse(authorIdString, out var authorUid)) | ||
| { | ||
| authorName = GetAuthorName(authorUid); |
There was a problem hiding this comment.
Nitpick: typo
| else if (UdiParser.TryParse(authorIdString, out var authorUid)) | |
| { | |
| authorName = GetAuthorName(authorUid); | |
| else if (UdiParser.TryParse(authorIdString, out var authorUdi)) | |
| { | |
| authorName = GetAuthorName(authorUdi); |
| var authorId = e.ValueSet.GetSingleValue<int?>(ExamineFields.PostAuthor); | ||
| var authorIdString = e.ValueSet.GetSingleValue(ExamineFields.PostAuthor); | ||
| var authorName = string.Empty; | ||
| if(int.TryParse(authorIdString, out var authorId)) |
There was a problem hiding this comment.
Nitpick/readability: Maybe to make this clearer for future code-readers, we could put this into a local method?
| if(int.TryParse(authorIdString, out var authorId)) | |
| if(IsUmbracoUserId(authorIdString, out var authorId)) |
| { | ||
| authorName = GetUserName(authorId); | ||
| } | ||
| else if (UdiParser.TryParse(authorIdString, out var authorUid)) |
There was a problem hiding this comment.
Nitpick/readability: Maybe to make this clearer for future code-readers, we could put this into a local method?
| else if (UdiParser.TryParse(authorIdString, out var authorUid)) | |
| else if (IsContentNodeUdi(authorIdString, out var authorUdi)) |
| { | ||
| values.Add(ExamineFields.Tag, tags.Select(t => t.ToLower())); | ||
| // Tags are likely csv rather than JSON |
There was a problem hiding this comment.
Good question. I think it explains what's going on cuz it might seem a bit weird that we try to deserialize this value, then do a string.Split() - makes it known this is intentional.
Don't feel strongly about it either way though
| private string GetAuthorName(Udi authorUid) | ||
| { | ||
| using (var context = _umbracoContextFactory.EnsureUmbracoContext()) | ||
| { | ||
| return context.UmbracoContext.Content.GetById(authorUid)?.Name ?? string.Empty; |
There was a problem hiding this comment.
Nitpick/Typo:
| private string GetAuthorName(Udi authorUid) | |
| { | |
| using (var context = _umbracoContextFactory.EnsureUmbracoContext()) | |
| { | |
| return context.UmbracoContext.Content.GetById(authorUid)?.Name ?? string.Empty; | |
| private string GetAuthorName(Udi authorUdi) | |
| { | |
| using (var context = _umbracoContextFactory.EnsureUmbracoContext()) | |
| { | |
| return context.UmbracoContext.Content.GetById(authorUdi)?.Name ?? string.Empty; |
| public string BlogPostDocumentTypeAlias => _settingsService.GetSetting("Gibe.Umbraco.Blog.BlogPostDocumentTypeAlias", "blogPost"); | ||
| public string BlogSectionDocumentTypeAlias => _settingsService.GetSetting("Gibe.Umbraco.Blog.BlogSectionDocumentTypeAlias", "blogSection"); | ||
|
|
||
| public string UserPickerPropertyEditorAlias => _settingsService.GetSetting("Gibe.Umbraco.Blog.UserPickerPropertyEditorAlias", "Umbraco.UserPicker"); |
There was a problem hiding this comment.
Nitpick: Seems strange this class is called "HardCodedBlogSettings" and then doesn't use hard-coded values - these don't seem very hard-coded to me! Should the class be renamed for framework? Maybe GibeSettingsBlogSettings?
Don't have hugely strong opinions either way on this though.
tristanjthompson
force-pushed
the
v10/feature/beyond-retail-blog-migration
branch
from
ca92c6d to
4e68dc8
Compare
tristanjthompson
force-pushed
the
v10/feature/beyond-retail-blog-migration
branch
from
4e68dc8 to
a1f3700
Compare
Migrated package so that it can be used in .NET core, as part of the BR blog migration.