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.

Sign up for GitHub

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

Jump to bottom

feat(blog): warn duplicate and inline authors #10224

Merged
merged 23 commits into from
Jun 27, 2024
Merged

feat(blog): warn duplicate and inline authors #10224

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
1eb2af6
feat(blog): warn duplicate and inline authors
OzakIOne Jun 17, 2024
373ab74
fix tests
OzakIOne Jun 17, 2024
5833974
fix test
OzakIOne Jun 17, 2024
8d71167
feat: tests
OzakIOne Jun 21, 2024
83bf814
refactor
OzakIOne Jun 21, 2024
7f59a6e
feat: tests
OzakIOne Jun 21, 2024
4e33812
refactor: apply lint autofix
OzakIOne Jun 21, 2024
dde8cde
ignore inline on duplicate check
OzakIOne Jun 21, 2024
ea1d1b7
refactor: apply lint autofix
OzakIOne Jun 21, 2024
4795cd0
remove typo
slorber Jun 26, 2024
b0eca61
Merge branch 'main' into ozaki/inlineAuthorWarn
slorber Jun 26, 2024
61d165f
remove typo
slorber Jun 26, 2024
aff176c
refactor authors duplicate algo
slorber Jun 26, 2024
534a7e9
remove inline attribute in favor of key
slorber Jun 26, 2024
564f2b5
extract and refactor logic to authorsProblems.ts
slorber Jun 26, 2024
9a487bc
improve tests for reportAuthorProblems
slorber Jun 26, 2024
4586ade
fix website blog author warnings
slorber Jun 26, 2024
22ea279
fix website blog author warnings
slorber Jun 26, 2024
4ef5096
fix website blog author warnings + polish
slorber Jun 26, 2024
042eaaf
revert useless tests changes
slorber Jun 26, 2024
9cbabbe
revert useless tests changes
slorber Jun 26, 2024
e4af303
revert useless diff
slorber Jun 26, 2024
489c3e0
fix tests
slorber Jun 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions packages/docusaurus-plugin-content-blog/src/__tests__/authors.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,14 @@ describe('getBlogPostAuthors', () => {
authorsMap: undefined,
baseUrl: '/',
}),
).toEqual([{name: 'Sébastien Lorber', title: 'maintainer'}]);
).toEqual([
{
name: 'Sébastien Lorber',
title: 'maintainer',
imageURL: undefined,
inline: true,
},
]);
});

it('can read authors Author[]', () => {
Expand All @@ -218,8 +225,17 @@ describe('getBlogPostAuthors', () => {
baseUrl: '/',
}),
).toEqual([
{name: 'Sébastien Lorber', title: 'maintainer'},
{name: 'Yangshun Tay'},
{
name: 'Sébastien Lorber',
title: 'maintainer',
imageURL: undefined,
inline: true,
},
{
name: 'Yangshun Tay',
imageURL: undefined,
inline: true,
},
]);
});

Expand Down Expand Up @@ -250,8 +266,9 @@ describe('getBlogPostAuthors', () => {
name: 'Yangshun Tay',
title: 'Yangshun title local override',
extra: 42,
imageURL: undefined,
},
{name: 'Alexey'},
{name: 'Alexey', inline: true, imageURL: undefined},
]);
});

Expand Down
2 changes: 2 additions & 0 deletions packages/docusaurus-plugin-content-blog/src/__tests__/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,9 +220,11 @@ describe('blog plugin', () => {
authors: [
{
name: 'Yangshun Tay (translated)',
inline: true,
},
{
email: 'lorber.sebastien@gmail.com',
imageURL: undefined,
key: 'slorber',
name: 'Sébastien Lorber (translated)',
title: 'Docusaurus maintainer (translated)',
Expand Down
36 changes: 35 additions & 1 deletion packages/docusaurus-plugin-content-blog/src/authors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import {getDataFileData, normalizeUrl} from '@docusaurus/utils';
import {Joi, URISchema} from '@docusaurus/utils-validation';
import logger from '@docusaurus/logger';
import type {BlogContentPaths} from './types';
import type {
Author,
Expand Down Expand Up @@ -123,6 +124,11 @@ function normalizeFrontMatterAuthors(
// we only support keys, otherwise, a typo in a key would fallback to
// becoming a name and may end up unnoticed
return {key: authorInput};
} else if (typeof authorInput === 'object' && !('key' in authorInput)) {
return {
...authorInput,
inline: true,
};
}
return authorInput;
}
Expand Down Expand Up @@ -171,7 +177,7 @@ ${Object.keys(authorsMap)
function fixAuthorImageBaseURL(
authors: Author[],
{baseUrl}: {baseUrl: string},
) {
): Author[] {
return authors.map((author) => ({
...author,
imageURL: normalizeImageUrl({imageURL: author.imageURL, baseUrl}),
Expand All @@ -196,5 +202,33 @@ Don't mix 'authors' with other existing 'author_*' front matter. Choose one or t
return [authorLegacy];
}

const inlineAuthors = updatedAuthors.filter((author) => author.inline);

const duplicateList = updatedAuthors.filter(
(author, index, self) =>
index !== self.findIndex((t) => t.name === author.name),
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Author might have no name

What happens if we have 2 authors with just an image? Are they considered as duplicate?

What happens if one predefined author has key "seb" and name "Seb", and there's another inline author named "Seb", are they considered as duplicate?

The current implementation is not good enough, and the method to detect duplicates is complex enough to deserve its own unit tests

Copy link
Collaborator

Choose a reason for hiding this comment

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

This case should work and not warn

https://deploy-preview-10224--docusaurus-2.netlify.app/tests/blog/2022/01/20/image-only-authors/

CleanShot 2024-06-20 at 16 42 35


// TODO need title check otherwise reports weird cases
if (inlineAuthors.length > 0 && params.frontMatter.title) {
logger.warn(
`Inline authors found in blog [${
params.frontMatter.title
}] ${inlineAuthors.map((author) => author.name).join(', ')}`,
);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need an option onInlineAuthors, it must be configurable because most blogs use inline authors atm.

// TODO need title check otherwise reports weird cases

What are those cases exactly? We need to handle those and cover with unit test edge cases

Overall the code should look quite similar to the tags code:

export function reportInlineTags({
  tags,
  source,
  options,
}: {
  tags: TagMetadata[];
  source: string;
  options: TagsPluginOptions;
}): void {
  if (options.onInlineTags === 'ignore') {
    return;
  }
  const inlineTags = tags.filter((tag) => tag.inline);
  if (inlineTags.length > 0) {
    const uniqueUnknownTags = [...new Set(inlineTags.map((tag) => tag.label))];
    const tagListString = uniqueUnknownTags.join(', ');
    logger.report(options.onInlineTags)(
      `Tags [${tagListString}] used in ${source} are not defined in ${
        options.tags ?? 'tags.yml'
      }`,
    );
  }
}

I'd prefer if the side effect (logging) wasn't performed inside getAuthors but as a separate standalone dedicated method, tested independently.

  const authors = getBlogPostAuthors({authorsMap, frontMatter, baseUrl});
  reportAuthorProblems(paramsYouNeed)

If logging requires more info about the blog (like the title/source/whatever) you can inject more as params.


// TODO need title check otherwise reports weird cases
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reports authors from changelog, maybe there is a better way to ignore that

if (duplicateList.length > 0 && params.frontMatter.title) {
OzakIOne marked this conversation as resolved.
Show resolved Hide resolved
console.log('duplicateList', duplicateList);
logger.error(
`Duplicate authors found in blog post ${params.frontMatter.title} [${
params.frontMatter.slug
}] front matter: ${duplicateList
.map((author) => author.name)
.join(', ')}`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

front matter might have no title, no slug, and author might have no name

All these cases should be covered with unit tests

);
}

return updatedAuthors;
}