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.

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

CLDR-17776 disallow reports if vetting closed #3840

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading