Skip to content

Commit

Permalink
CLDR-17560 UI for entire locale checks (#3644)
Browse files Browse the repository at this point in the history
  • Loading branch information
srl295 authored May 2, 2024
1 parent e89f253 commit 1f9e557
Show file tree
Hide file tree
Showing 12 changed files with 156 additions and 6 deletions.
14 changes: 13 additions & 1 deletion tools/cldr-apps/js/src/esm/cldrSurvey.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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 += "<p class='trInfo tr_" + testItem.type;
if (testItem.type == "Warning") {
newHtml += " alert alert-warning fix-popover-help";
Expand All @@ -780,7 +787,12 @@ function testsToHtml(tests) {
newHtml += ' <a href="' + testItem.subtypeUrl + '">(how to fix…)</a>';
}

newHtml += "</p>";
newHtml += "</p>\n";
}
if (hadEntireLocale) {
newHtml += `<p class='trInfo tr_Warning alert alert-warning fix-popover-help'>See also <a href='#r_supplemental/${cldrStatus.getCurrentLocale()}//'>${cldrText.get(
"special_r_supplemental"
)}</a></p>\n`;
}
return newHtml;
}
Expand Down
1 change: 1 addition & 0 deletions tools/cldr-apps/js/src/esm/cldrText.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -786,6 +787,12 @@ protected boolean accept(List<CheckStatus> 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<Subtype, String> {}

/** Status value returned from check */
public static class CheckStatus implements Comparable<CheckStatus> {
public static final Type alertType = Type.Comment,
Expand Down Expand Up @@ -1126,7 +1133,7 @@ public static boolean hasType(List<CheckStatus> 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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,7 @@ public static void main(String[] args) throws Throwable {
boolean showedOne = false;
for (Iterator<CheckStatus> it3 = result.iterator(); it3.hasNext(); ) {
CheckStatus status = it3.next();
if (status.isEntireLocale()) {
if (status.getEntireLocale()) {
possibleProblems.add(status);
continue;
}
Expand Down
16 changes: 16 additions & 0 deletions tools/cldr-code/src/main/java/org/unicode/cldr/tool/Chart.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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 "<p>This chart shows errors which apply to the entire locale.</p>";
}

@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<CheckStatus> pp = new TreeSet<CheckStatus>();

// add any 'early' errors
pp.addAll(test.getPossibleProblems());

// add all non-path status
for (final String x : cldrFile) {
List<CheckStatus> result = new ArrayList<CheckStatus>();
test.check(x, result, cldrFile.getStringValue(x));
for (final CheckStatus s : result) {
if (s.getEntireLocale()) pp.add(s);
}
}

// report

if (pp.isEmpty()) {
pw.write("<h3>No Errors</h3>\n");
pw.write("There are no overall locale issues to report");
} else {
pw.write("<h3>Overall Errors</h3>\n");
pw.write("<ol>\n");
for (final CheckStatus s : pp) {
pw.write(
String.format(
"<li> <b>%s</b> <i title='%s'>%s</i>\n",
s.getType(), s.getSubtype().name(), s.getSubtype()));
pw.write("<p>" + s.getMessage() + "</p>");
if (urlProvider != null) {
final String moreDetailsUrl = urlProvider.apply(s.getSubtype());
pw.write(String.format("<a href=\"%s\">more details</a>", moreDetailsUrl));
}
pw.write("</li>\n");
}
pw.write("</ol>\n\n");
}
}

pw.write("</div> <!-- ReportSupplemental -->\n");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.getEntireLocale())
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ public abstract class VoterReportStatus<T> {
* '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'
Expand Down

0 comments on commit 1f9e557

Please sign in to comment.