fix(plugin-manifest-validator): return warning message if homepage_url or name is empty

[hung-cybo]

Why

Currently, the homepage_url property of manifest.json is optional. When kintone users encounter trouble and no homepage_url in the plugin, users tend to contact Cybozu support.

Therefore, in this PR, warning messages will be returned if the name of the specific language is specified, but the homepage_url for that language is empty, and vice versa.

What

• Return warning messages if the name of the specific language is specified, but the homepage_url for that language is empty, and vice versa.

How to test

pnpm test

Checklist

• Read CONTRIBUTING.md
• Updated documentation if it is required.
• Added tests if it is required.
• Passed pnpm lint and pnpm test on the root directory.

[aws-amplify-ap-northeast-1]

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-2776.d1ite8zvzg8c5i.amplifyapp.com

[tuanphamcybozu]

@hung-cybo
Split this condition into a seperate function can be:

• validateRequiredProperties is shorten
• easy to understand context
• easy for refactoring and writing unit test for the condition

[tuanphamcybozu]

@hung-cybo
is it better to move this function to another place?
Similar as:

js-sdk/packages/plugin-manifest-validator/src/validate-https-url.ts

Line 8 in 5a0b859

const validateHttpsUrl = (

[tuanphamcybozu]

@hung-cybo
Reversing this condition to make an early return, and avoid a nested if.

if (errors.length === 0) {
    return true;
}

if (schema.warn) {
....

[tuanphamcybozu]

@hung-cybo
Splitting this function to the outside of checkRequiredProperties function can improve code readability, reusability, and testing. Further more, this makes checkRequiredProperties shorter.

[tuanphamcybozu]

@hung-cybo
How about splitting into several functions?
Such as

....
....
  schema.items.forEach(item => {
      if (typeof item === "object") {
          validateObjectItem(item, json, errors, schema.warn);
      } else {
          validatePrimitiveItem(item, json, errors, schema.warn);
      }
 });

const validateObjectItem = (item, json, errors, warn) => {
    Object.keys(item).forEach(property => {
    if (!item[property].properties || item[property].properties.length === 0) {
      return;
    }

    validateProperty(item[property], property, json, errors, warn);
  });
}

const validateProperty =  (propertyDetail, propertyName, json, errors, warn) => {
    propertyDetail.properties.forEach(prop => {
    if (!json[propertyName] || !json[propertyName][prop]) {
      errors.push(generateErrorMessage(`${propertyName}.${prop}`, warn));
    }
  });
}

const validatePrimitiveItem = (item, json, errors, warn) => {
    if (!json[item]) {
    errors.push(generateErrorMessage(item, warn));
  }
}

P/s: sorry this snippet code doesn't have type and test.

[tuanphamcybozu]

LGTM

[tasshi-me]

Isn't it better to create a warning object like the following? (for future expansion)

type WarningObject {
  message: string
}

[tasshi-me]

LGTM!