Skip to content

Commit

Permalink
CLDR-17776 disallow reports if vetting closed (#3840)
Browse files Browse the repository at this point in the history
  • Loading branch information
srl295 authored Jul 2, 2024
1 parent 61da87d commit 2ea95b1
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 31 deletions.
8 changes: 4 additions & 4 deletions tools/cldr-apps/js/src/esm/cldrReport.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ async function getOneLocaleStatus(locale) {
`getOneLocaleStatus(${locale}) expected an array of one item but got ${obj.locales.length}`
);
}
return obj.locales[0].reports;
return obj.locales[0];
}

/**
Expand All @@ -122,9 +122,9 @@ async function getOneLocaleStatus(locale) {
* @returns
*/
async function getOneReportLocaleStatus(locale, onlyReport) {
const reports = await getOneLocaleStatus(locale);
const myReport = reports.filter(({ report }) => report === onlyReport)[0];
return myReport;
const { reports, canVote } = await getOneLocaleStatus(locale);
const report = reports.filter(({ report }) => report === onlyReport)[0];
return { report, canVote };
}

/**
Expand Down
23 changes: 18 additions & 5 deletions tools/cldr-apps/js/src/views/ReportResponse.vue
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
before continuing.
</p>

<a-radio-group v-model:value="state" @change="changed">
<a-radio-group v-model:value="state" :disabled="!canVote" @change="changed">
<a-radio :style="radioStyle" value="acceptable">
I have reviewed the items below, and they are all acceptable</a-radio
>
Expand Down Expand Up @@ -44,6 +44,7 @@

<script>
import * as cldrClient from "../esm/cldrClient.mjs";
import * as cldrNotify from "../esm/cldrNotify.mjs";
import * as cldrReport from "../esm/cldrReport.mjs";
import * as cldrStatus from "../esm/cldrStatus.mjs";
import * as cldrTable from "../esm/cldrTable.mjs";
Expand All @@ -59,6 +60,7 @@ export default {
],
data() {
return {
canVote: false,
completed: false,
acceptable: false,
loaded: false,
Expand Down Expand Up @@ -172,7 +174,12 @@ export default {
);
await this.reload(); // will set loaded=true
} catch (e) {
console.error(e);
cldrNotify.exception(
e,
`Trying to vote for report ${
this.report
} in ${cldrStatus.getCurrentLocale()}`
);
this.error = e;
this.loaded = true;
}
Expand Down Expand Up @@ -205,12 +212,18 @@ export default {
completed: this.completed,
acceptable: this.acceptable,
});
this.reportStatus = await reportLocaleStatusResponse; // { status: approved, acceptability: acceptable }
console.dir(await reportLocaleStatusResponse);
const { report, canVote } = await reportLocaleStatusResponse;
this.reportStatus = report; // { status: approved, acceptability: acceptable }
this.loaded = true;
this.canVote = canVote;
this.error = null;
} catch (e) {
console.error(e);
cldrNotify.exception(
e,
`Trying to load report ${
this.report
} in ${cldrStatus.getCurrentLocale()}`
);
this.error = e;
this.loaded = true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.eclipse.microprofile.openapi.annotations.responses.APIResponse;
import org.eclipse.microprofile.openapi.annotations.responses.APIResponses;
import org.eclipse.microprofile.openapi.annotations.tags.Tag;
import org.unicode.cldr.test.CheckCLDR;
import org.unicode.cldr.tool.Chart;
import org.unicode.cldr.util.CLDRLocale;
import org.unicode.cldr.util.VoteResolver;
Expand Down Expand Up @@ -190,7 +191,15 @@ public Response getReportLocaleStatus(
// set of all valid userids
final Set<Integer> allUsers = CookieSession.sm.reg.getVoterToInfo().keySet();
for (final CLDRLocale loc : locales) {
LocaleReportVettingResult rr = new LocaleReportVettingResult();
CheckCLDR.Phase phase = SurveyMain.checkCLDRPhase(loc);
CheckCLDR.StatusAction showRowAction =
phase.getShowRowAction(
null /* not path based */,
CheckCLDR.InputMethod.DIRECT,
null /* Not path based */,
mySession.user);
final boolean canModify = UserRegistry.userCanModifyLocale(mySession.user, loc);
LocaleReportVettingResult rr = new LocaleReportVettingResult(showRowAction, canModify);
rr.locale = loc.toString();
for (final ReportId report : ReportId.getReportsAvailable()) {
Map<Integer, ReportAcceptability> votes = new TreeMap<>();
Expand All @@ -215,6 +224,9 @@ public LocaleReportVettingResult[] getLocales() {
}

public static class LocaleReportVettingResult {
@Schema(description = "True if user is allowed to vote for this report.")
public final boolean canVote;

public String locale;
private Set<ReportVettingResult> reports = new HashSet<ReportVettingResult>();

Expand All @@ -232,6 +244,10 @@ void addVoters(Set<Integer> s) {
public int getTotalVoters() {
return allUsers.size();
}

public LocaleReportVettingResult(CheckCLDR.StatusAction action, boolean canModify) {
canVote = canModify && action.canVote();
}
}

public static class ReportVettingResult {
Expand Down Expand Up @@ -327,13 +343,26 @@ public Response updateReport(
if (!report.isAvailable()) {
return Response.status(Status.FORBIDDEN).build();
}
final CLDRLocale loc = CLDRLocale.getInstance(locale);
// apply the same standard as for vetting.
// First check whether they even have permission.
if (!UserRegistry.userCanModifyLocale(mySession.user, loc)) {
return Response.status(Status.FORBIDDEN).build();
}
CheckCLDR.StatusAction showRowAction =
SurveyMain.checkCLDRPhase(loc)
.getShowRowAction(
null /* not path based */,
CheckCLDR.InputMethod.DIRECT,
null /* Not path based */,
mySession.user);

if (!showRowAction.canVote()) {
return Response.status(Status.FORBIDDEN).build();
}

ReportsDB.getInstance()
.markReportComplete(
user,
CLDRLocale.getInstance(locale),
report,
update.completed,
update.acceptable);
.markReportComplete(user, loc, report, update.completed, update.acceptable);

return Response.status(Status.NO_CONTENT).build();
}
Expand Down
50 changes: 35 additions & 15 deletions tools/cldr-code/src/main/java/org/unicode/cldr/test/CheckCLDR.java
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,16 @@ public boolean isForbidden() {
public boolean canShow() {
return !isForbidden;
}

public boolean canVote() {
// the one non-voting case
if (this == ALLOW_TICKET_ONLY) return false;
return !isForbidden();
}

public boolean canSubmit() {
return (this == ALLOW);
}
}

private static final HashMap<String, Phase> PHASE_NAMES = new HashMap<>();
Expand Down Expand Up @@ -195,9 +205,9 @@ public boolean isUnitTest() {
/**
* Return whether or not to show a row, and if so, how.
*
* @param pathValueInfo
* @param pathValueInfo - may be null for a non-path entry.
* @param inputMethod
* @param ph the path header
* @param ph the path header - may be null if it is a non-path entry
* @param userInfo null if there is no userInfo (nobody logged in).
* @return
*/
Expand All @@ -208,7 +218,14 @@ public StatusAction getShowRowAction(
UserInfo userInfo // can get voterInfo from this.
) {

PathHeader.SurveyToolStatus status = ph.getSurveyToolStatus();
// default to read/write
PathHeader.SurveyToolStatus status = PathHeader.SurveyToolStatus.READ_WRITE;
boolean canReadAndWrite = true;

if (ph != null) {
status = ph.getSurveyToolStatus();
canReadAndWrite = ph.canReadAndWrite();
}
/*
* Always forbid DEPRECATED items - don't show.
*
Expand Down Expand Up @@ -239,23 +256,26 @@ public StatusAction getShowRowAction(
return StatusAction.FORBID_READONLY;
}

CandidateInfo winner = pathValueInfo.getCurrentItem();
ValueStatus valueStatus = getValueStatus(winner, ValueStatus.NONE, null);
ValueStatus valueStatus = ValueStatus.NONE;
if (pathValueInfo != null) {
CandidateInfo winner = pathValueInfo.getCurrentItem();
valueStatus = getValueStatus(winner, ValueStatus.NONE, null);

// if limited submission, and winner doesn't have an error, limit the values
// if limited submission, and winner doesn't have an error, limit the values

if (LIMITED_SUBMISSION) {
if (!SubmissionLocales.allowEvenIfLimited(
pathValueInfo.getLocale().toString(),
pathValueInfo.getXpath(),
valueStatus == ValueStatus.ERROR,
pathValueInfo.getBaselineStatus() == Status.missing)) {
return StatusAction.FORBID_READONLY;
if (LIMITED_SUBMISSION) {
if (!SubmissionLocales.allowEvenIfLimited(
pathValueInfo.getLocale().toString(),
pathValueInfo.getXpath(),
valueStatus == ValueStatus.ERROR,
pathValueInfo.getBaselineStatus() == Status.missing)) {
return StatusAction.FORBID_READONLY;
}
}
}

if (this == Phase.SUBMISSION || isUnitTest()) {
return (ph.canReadAndWrite())
return (canReadAndWrite)
? StatusAction.ALLOW
: StatusAction.ALLOW_VOTING_AND_TICKET;
}
Expand All @@ -265,7 +285,7 @@ public StatusAction getShowRowAction(
// Only allow ADD if we have an error or warning
// Only check winning value for errors/warnings per ticket #8677
if (valueStatus != ValueStatus.NONE) {
return (ph.canReadAndWrite())
return (canReadAndWrite)
? StatusAction.ALLOW
: StatusAction.ALLOW_VOTING_AND_TICKET;
}
Expand Down

0 comments on commit 2ea95b1

Please sign in to comment.