From ddd06547237fd806a182bf06b4f6b6a14bc3d59a Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Mon, 22 Apr 2024 19:21:17 -0500 Subject: [PATCH 1/2] CLDR-17560 CheckCLDR UI for non-path checks - add a new 'supplemental' report that simply lists the overall errors - update Chart API to take testbundle and subtype mapper --- tools/cldr-apps/js/src/esm/cldrText.mjs | 1 + .../org/unicode/cldr/web/SubtypeToURLMap.java | 8 +- .../org/unicode/cldr/web/api/ReportAPI.java | 10 +- .../java/org/unicode/cldr/test/CheckCLDR.java | 7 ++ .../java/org/unicode/cldr/tool/Chart.java | 16 ++++ .../unicode/cldr/tool/ChartSupplemental.java | 96 +++++++++++++++++++ .../org/unicode/cldr/util/VettingViewer.java | 2 + .../unicode/cldr/util/VoterReportStatus.java | 1 + 8 files changed, 138 insertions(+), 3 deletions(-) create mode 100644 tools/cldr-code/src/main/java/org/unicode/cldr/tool/ChartSupplemental.java diff --git a/tools/cldr-apps/js/src/esm/cldrText.mjs b/tools/cldr-apps/js/src/esm/cldrText.mjs index e4de7986805..b27a54ebcf0 100644 --- a/tools/cldr-apps/js/src/esm/cldrText.mjs +++ b/tools/cldr-apps/js/src/esm/cldrText.mjs @@ -495,6 +495,7 @@ const strings = { special_r_datetime: "Datetime", special_r_zones: "Zones", special_r_personnames: "Person Names", + special_r_supplemental: "Entire Locale Errors", special_recent_activity: "Recent Activity", special_retry: "Retry", special_retry_inplace: "Retry", diff --git a/tools/cldr-apps/src/main/java/org/unicode/cldr/web/SubtypeToURLMap.java b/tools/cldr-apps/src/main/java/org/unicode/cldr/web/SubtypeToURLMap.java index b6ed8634989..3b3cdc41591 100644 --- a/tools/cldr-apps/src/main/java/org/unicode/cldr/web/SubtypeToURLMap.java +++ b/tools/cldr-apps/src/main/java/org/unicode/cldr/web/SubtypeToURLMap.java @@ -36,13 +36,14 @@ import org.jsoup.Jsoup; import org.jsoup.nodes.Document; import org.unicode.cldr.test.CheckCLDR.CheckStatus.Subtype; +import org.unicode.cldr.test.CheckCLDR.SubtypeToURLProvider; import org.unicode.cldr.util.CLDRCacheDir; import org.unicode.cldr.util.CLDRTool; @CLDRTool( alias = "subtype-to-url-map", description = "parse each of the params as a path or URL to a subtype map and check.") -public class SubtypeToURLMap { +public class SubtypeToURLMap implements SubtypeToURLProvider { static final Logger logger = SurveyLog.forClass(SubtypeToURLMap.class); /** * Little tool for validating input data. @@ -514,4 +515,9 @@ public static String forSubtype(Subtype subtype) { if (SubtypeToURLMapHelper.INSTANCE == null) return null; return SubtypeToURLMapHelper.INSTANCE.get(subtype); } + + @Override + public String apply(Subtype t) { + return forSubtype(t); + } } diff --git a/tools/cldr-apps/src/main/java/org/unicode/cldr/web/api/ReportAPI.java b/tools/cldr-apps/src/main/java/org/unicode/cldr/web/api/ReportAPI.java index 147d8f3d6f4..7973d09ecc0 100644 --- a/tools/cldr-apps/src/main/java/org/unicode/cldr/web/api/ReportAPI.java +++ b/tools/cldr-apps/src/main/java/org/unicode/cldr/web/api/ReportAPI.java @@ -32,9 +32,11 @@ import org.unicode.cldr.util.VoterReportStatus.ReportAcceptability; import org.unicode.cldr.util.VoterReportStatus.ReportId; import org.unicode.cldr.web.CookieSession; +import org.unicode.cldr.web.DataPage; import org.unicode.cldr.web.ReportsDB; import org.unicode.cldr.web.ReportsDB.UserReport; import org.unicode.cldr.web.STFactory; +import org.unicode.cldr.web.SubtypeToURLMap; import org.unicode.cldr.web.SurveyAjax; import org.unicode.cldr.web.SurveyMain; import org.unicode.cldr.web.UserRegistry; @@ -414,10 +416,14 @@ public Response getReportOutput( private String writeReport(ReportId report, CLDRLocale loc) throws IOException { final Writer w = new StringWriter(); + final STFactory stf = CookieSession.sm.getSTFactory(); final Chart chart = Chart.forReport(report, loc.getBaseName()); if (chart != null) { - final STFactory stf = CookieSession.sm.getSTFactory(); - chart.writeContents(w, stf); + chart.writeContents( + w, + stf, + stf.getTestResult(loc, DataPage.getSimpleOptions(loc)), + SubtypeToURLMap.getInstance()); } else { switch (report) { // "Old Three" reports diff --git a/tools/cldr-code/src/main/java/org/unicode/cldr/test/CheckCLDR.java b/tools/cldr-code/src/main/java/org/unicode/cldr/test/CheckCLDR.java index 18588668af8..182f727a653 100644 --- a/tools/cldr-code/src/main/java/org/unicode/cldr/test/CheckCLDR.java +++ b/tools/cldr-code/src/main/java/org/unicode/cldr/test/CheckCLDR.java @@ -26,6 +26,7 @@ import java.util.Map; import java.util.Set; import java.util.TreeSet; +import java.util.function.Function; import java.util.logging.Logger; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -786,6 +787,12 @@ protected boolean accept(List result) { Options cachedOptions = null; + /** + * abstract interface for mapping from a Subtype to a "more details" URL. see + * org.unicode.cldr.web.SubtypeToURLMap + */ + public interface SubtypeToURLProvider extends Function {} + /** Status value returned from check */ public static class CheckStatus implements Comparable { public static final Type alertType = Type.Comment, diff --git a/tools/cldr-code/src/main/java/org/unicode/cldr/tool/Chart.java b/tools/cldr-code/src/main/java/org/unicode/cldr/tool/Chart.java index 2c34553da69..080e361c8ad 100644 --- a/tools/cldr-code/src/main/java/org/unicode/cldr/tool/Chart.java +++ b/tools/cldr-code/src/main/java/org/unicode/cldr/tool/Chart.java @@ -9,6 +9,8 @@ import java.io.Writer; import java.util.Arrays; import java.util.stream.Collectors; +import org.unicode.cldr.test.CheckCLDR; +import org.unicode.cldr.test.TestCache.TestResultBundle; import org.unicode.cldr.tool.FormattedFileWriter.Anchors; import org.unicode.cldr.util.CLDRConfig; import org.unicode.cldr.util.CLDRFile; @@ -158,6 +160,18 @@ public void writeContents(Writer pw, Factory factory) throws IOException { throw new IllegalArgumentException("Not implemented yet"); } + /** + * extended function with some additional parameters subclasses may optionally implement this. + */ + public void writeContents( + Writer pw, + Factory factory, + TestResultBundle bundle, + CheckCLDR.SubtypeToURLProvider urlProvider) + throws IOException { + this.writeContents(pw, factory); + } + private static final class AnalyticsHelper { private static final AnalyticsHelper INSTANCE = new AnalyticsHelper(); @@ -220,6 +234,8 @@ public static Chart forReport(final ReportId report, final String locale) { switch (report) { case personnames: return new ChartPersonName(locale); + case supplemental: + return new ChartSupplemental(locale); default: return null; } diff --git a/tools/cldr-code/src/main/java/org/unicode/cldr/tool/ChartSupplemental.java b/tools/cldr-code/src/main/java/org/unicode/cldr/tool/ChartSupplemental.java new file mode 100644 index 00000000000..e8059ed25b0 --- /dev/null +++ b/tools/cldr-code/src/main/java/org/unicode/cldr/tool/ChartSupplemental.java @@ -0,0 +1,96 @@ +package org.unicode.cldr.tool; + +import java.io.IOException; +import java.io.Writer; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; +import org.unicode.cldr.test.CheckCLDR.CheckStatus; +import org.unicode.cldr.test.CheckCLDR.SubtypeToURLProvider; +import org.unicode.cldr.test.TestCache.TestResultBundle; +import org.unicode.cldr.util.CLDRConfig; +import org.unicode.cldr.util.CLDRFile; +import org.unicode.cldr.util.CLDRPaths; +import org.unicode.cldr.util.Factory; + +public class ChartSupplemental extends Chart { + private static final CLDRConfig CLDR_CONFIG = CLDRConfig.getInstance(); + static final CLDRFile ENGLISH = CLDR_CONFIG.getEnglish(); + static final String DIR = CLDRPaths.CHART_DIRECTORY + "verify/supplemental/"; + + private final String locale; + + public ChartSupplemental(String locale) { + super(); + this.locale = locale; + } + + @Override + public String getDirectory() { + return DIR; + } + + @Override + public String getTitle() { + return ENGLISH.getName(locale) + ": Overall Errors"; + } + + @Override + public String getExplanation() { + return "

This chart shows errors which apply to the entire locale.

"; + } + + @Override + public String getFileName() { + return locale; + } + + @Override + public void writeContents( + Writer pw, Factory factory, TestResultBundle test, SubtypeToURLProvider urlProvider) + throws IOException { + CLDRFile cldrFile = factory.make(locale, true); + + if (test != null) { + Set pp = new TreeSet(); + + // add any 'early' errors + pp.addAll(test.getPossibleProblems()); + + // add all non-path status + for (final String x : cldrFile) { + List result = new ArrayList(); + test.check(x, result, cldrFile.getStringValue(x)); + for (final CheckStatus s : result) { + if (s.isEntireLocale()) pp.add(s); + } + } + + // report + + if (pp.isEmpty()) { + pw.write("

No Errors

\n"); + pw.write("There are no overall locale issues to report"); + } else { + pw.write("

Overall Errors

\n"); + pw.write("
    \n"); + for (final CheckStatus s : pp) { + pw.write( + String.format( + "
  1. %s %s\n", + s.getType(), s.getSubtype().name(), s.getSubtype())); + pw.write("

    " + s.getMessage() + "

    "); + if (urlProvider != null) { + final String moreDetailsUrl = urlProvider.apply(s.getSubtype()); + pw.write(String.format("more details", moreDetailsUrl)); + } + pw.write("
  2. \n"); + } + pw.write("
\n\n"); + } + } + + pw.write(" \n"); + } +} diff --git a/tools/cldr-code/src/main/java/org/unicode/cldr/util/VettingViewer.java b/tools/cldr-code/src/main/java/org/unicode/cldr/util/VettingViewer.java index 6696998312d..c6a6c830dea 100644 --- a/tools/cldr-code/src/main/java/org/unicode/cldr/util/VettingViewer.java +++ b/tools/cldr-code/src/main/java/org/unicode/cldr/util/VettingViewer.java @@ -197,6 +197,8 @@ public Status getErrorStatus( StringBuilder errorMessage = new StringBuilder(); factory.getTestCache().getBundle(options).check(path, result, value); for (CheckStatus checkStatus : result) { + if (checkStatus.isEntireLocale()) + continue; // these will show up in the Entire Locale (supplemental) report final CheckCLDR cause = checkStatus.getCause(); /* * CheckCoverage will be shown under Missing, not under Warnings; and diff --git a/tools/cldr-code/src/main/java/org/unicode/cldr/util/VoterReportStatus.java b/tools/cldr-code/src/main/java/org/unicode/cldr/util/VoterReportStatus.java index 73e669af078..2e4bd8d22bb 100644 --- a/tools/cldr-code/src/main/java/org/unicode/cldr/util/VoterReportStatus.java +++ b/tools/cldr-code/src/main/java/org/unicode/cldr/util/VoterReportStatus.java @@ -21,6 +21,7 @@ public abstract class VoterReportStatus { * 'Person Names' Also see {@link org.unicode.cldr.tool.Chart#forReport(ReportId, String)} */ public enum ReportId { + supplemental, // non-Chart (Entire Locale) datetime, // non-Chart zones, // non-Chart compact, // non-Chart, aka 'numbers' From 466bdcb73319c705dcf566adf877a4b54aba5051 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Fri, 26 Apr 2024 13:41:02 -0500 Subject: [PATCH 2/2] CLDR-17560 CheckCLDR UI for non-path checks - rollup non-path checks into a single warning in the info panel - fix Voting API to expose this data --- tools/cldr-apps/js/src/esm/cldrSurvey.mjs | 14 +++++++++++++- .../org/unicode/cldr/web/SurveyJSONWrapper.java | 1 + .../java/org/unicode/cldr/web/api/VoteAPI.java | 2 ++ .../main/java/org/unicode/cldr/test/CheckCLDR.java | 2 +- .../org/unicode/cldr/test/ConsoleCheckCLDR.java | 2 +- .../org/unicode/cldr/tool/ChartSupplemental.java | 2 +- .../java/org/unicode/cldr/util/VettingViewer.java | 2 +- 7 files changed, 20 insertions(+), 5 deletions(-) diff --git a/tools/cldr-apps/js/src/esm/cldrSurvey.mjs b/tools/cldr-apps/js/src/esm/cldrSurvey.mjs index 48d4591d11a..2d348f10316 100644 --- a/tools/cldr-apps/js/src/esm/cldrSurvey.mjs +++ b/tools/cldr-apps/js/src/esm/cldrSurvey.mjs @@ -752,8 +752,15 @@ function testsToHtml(tests) { if (!tests) { return newHtml; } + var hadEntireLocale = false; + for (var i = 0; i < tests.length; i++) { var testItem = tests[i]; + const { entireLocale } = testItem; + if (entireLocale) { + hadEntireLocale = true; + continue; + } newHtml += "

(how to fix…)'; } - newHtml += "

"; + newHtml += "

\n"; + } + if (hadEntireLocale) { + newHtml += `

See also ${cldrText.get( + "special_r_supplemental" + )}

\n`; } return newHtml; } diff --git a/tools/cldr-apps/src/main/java/org/unicode/cldr/web/SurveyJSONWrapper.java b/tools/cldr-apps/src/main/java/org/unicode/cldr/web/SurveyJSONWrapper.java index 90d340b4ee6..31c14886316 100644 --- a/tools/cldr-apps/src/main/java/org/unicode/cldr/web/SurveyJSONWrapper.java +++ b/tools/cldr-apps/src/main/java/org/unicode/cldr/web/SurveyJSONWrapper.java @@ -67,6 +67,7 @@ public static JSONObject wrap(CheckStatus status) throws JSONException { put("subtype", subtype.name()); put("subtypeUrl", SubtypeToURLMap.forSubtype(subtype)); // could be null. } + put("entireLocale", cs.getEntireLocale()); } }; } diff --git a/tools/cldr-apps/src/main/java/org/unicode/cldr/web/api/VoteAPI.java b/tools/cldr-apps/src/main/java/org/unicode/cldr/web/api/VoteAPI.java index 544f482d433..b0ec3882a92 100644 --- a/tools/cldr-apps/src/main/java/org/unicode/cldr/web/api/VoteAPI.java +++ b/tools/cldr-apps/src/main/java/org/unicode/cldr/web/api/VoteAPI.java @@ -352,6 +352,7 @@ public static final class CheckStatusSummary { public String subtypeUrl; public Phase phase; public String cause; + public boolean entireLocale; public CheckStatusSummary(CheckStatus checkStatus) { this.message = checkStatus.getMessage(); @@ -366,6 +367,7 @@ public CheckStatusSummary(CheckStatus checkStatus) { if (this.subtype != null) { this.subtypeUrl = SubtypeToURLMap.forSubtype(this.subtype); // could be null. } + this.entireLocale = checkStatus.getEntireLocale(); } } } diff --git a/tools/cldr-code/src/main/java/org/unicode/cldr/test/CheckCLDR.java b/tools/cldr-code/src/main/java/org/unicode/cldr/test/CheckCLDR.java index 182f727a653..d5caed38a89 100644 --- a/tools/cldr-code/src/main/java/org/unicode/cldr/test/CheckCLDR.java +++ b/tools/cldr-code/src/main/java/org/unicode/cldr/test/CheckCLDR.java @@ -1133,7 +1133,7 @@ public static boolean hasType(List result, Type type) { /** * @returns true if this status applies to the entire locale, not a single path */ - public boolean isEntireLocale() { + public boolean getEntireLocale() { return entireLocale; } diff --git a/tools/cldr-code/src/main/java/org/unicode/cldr/test/ConsoleCheckCLDR.java b/tools/cldr-code/src/main/java/org/unicode/cldr/test/ConsoleCheckCLDR.java index 506b30fd150..6bf84dfc85c 100644 --- a/tools/cldr-code/src/main/java/org/unicode/cldr/test/ConsoleCheckCLDR.java +++ b/tools/cldr-code/src/main/java/org/unicode/cldr/test/ConsoleCheckCLDR.java @@ -777,7 +777,7 @@ public static void main(String[] args) throws Throwable { boolean showedOne = false; for (Iterator it3 = result.iterator(); it3.hasNext(); ) { CheckStatus status = it3.next(); - if (status.isEntireLocale()) { + if (status.getEntireLocale()) { possibleProblems.add(status); continue; } diff --git a/tools/cldr-code/src/main/java/org/unicode/cldr/tool/ChartSupplemental.java b/tools/cldr-code/src/main/java/org/unicode/cldr/tool/ChartSupplemental.java index e8059ed25b0..2b287892939 100644 --- a/tools/cldr-code/src/main/java/org/unicode/cldr/tool/ChartSupplemental.java +++ b/tools/cldr-code/src/main/java/org/unicode/cldr/tool/ChartSupplemental.java @@ -63,7 +63,7 @@ public void writeContents( List result = new ArrayList(); test.check(x, result, cldrFile.getStringValue(x)); for (final CheckStatus s : result) { - if (s.isEntireLocale()) pp.add(s); + if (s.getEntireLocale()) pp.add(s); } } diff --git a/tools/cldr-code/src/main/java/org/unicode/cldr/util/VettingViewer.java b/tools/cldr-code/src/main/java/org/unicode/cldr/util/VettingViewer.java index c6a6c830dea..0b6b2b3dac0 100644 --- a/tools/cldr-code/src/main/java/org/unicode/cldr/util/VettingViewer.java +++ b/tools/cldr-code/src/main/java/org/unicode/cldr/util/VettingViewer.java @@ -197,7 +197,7 @@ public Status getErrorStatus( StringBuilder errorMessage = new StringBuilder(); factory.getTestCache().getBundle(options).check(path, result, value); for (CheckStatus checkStatus : result) { - if (checkStatus.isEntireLocale()) + if (checkStatus.getEntireLocale()) continue; // these will show up in the Entire Locale (supplemental) report final CheckCLDR cause = checkStatus.getCause(); /*