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-16369 Move VoteStatus enum from VettingViewer to VoteResolver #3344

Merged
merged 1 commit into from
Oct 17, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

import java.util.Map;
import org.unicode.cldr.util.Organization;
import org.unicode.cldr.util.VettingViewer.VoteStatus;
import org.unicode.cldr.util.VoteResolver;
import org.unicode.cldr.util.VoteResolver.VoteStatus;
import org.unicode.cldr.web.UserRegistry.User;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import org.unicode.cldr.util.*;
import org.unicode.cldr.util.VettingViewer.UsersChoice;
import org.unicode.cldr.util.VettingViewer.VoteStatus;
import org.unicode.cldr.util.VoteResolver.VoteStatus;
import org.unicode.cldr.web.UserRegistry.User;

public class STUsersChoice implements UsersChoice<Organization> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,40 +88,6 @@ public static OutdatedPaths getOutdatedPaths() {
private static PathHeader.Factory pathTransform;
private static final OutdatedPaths outdatedPaths = new OutdatedPaths();

/** See VoteResolver getStatusForOrganization to see how this is computed. */
public enum VoteStatus {
/**
* The value for the path is either contributed or approved, and the user's organization
* didn't vote. (see class def for null user)
*/
ok_novotes,

/**
* The value for the path is either contributed or approved, and the user's organization
* chose the winning value. (see class def for null user)
*/
ok,

/**
* The user's organization chose the winning value for the path, but that value is neither
* contributed nor approved. (see class def for null user)
*/
provisionalOrWorse,

/**
* The user's organization's choice is not winning. There may be insufficient votes to
* overcome a previously approved value, or other organizations may be voting against it.
* (see class def for null user)
*/
losing,

/**
* There is a dispute, meaning more than one item with votes, or the item with votes didn't
* win.
*/
disputed
}

/**
* @author markdavis
* @param <T>
Expand All @@ -139,7 +105,8 @@ public interface UsersChoice<T> {
* Return the vote status NOTE: if organization = null, then it must disregard the
* organization and never return losing. See VoteStatus.
*/
VoteStatus getStatusForUsersOrganization(CLDRFile cldrFile, String path, T organization);
VoteResolver.VoteStatus getStatusForUsersOrganization(
CLDRFile cldrFile, String path, T organization);

/**
* Has the given user voted for the given path and locale?
Expand Down Expand Up @@ -530,9 +497,9 @@ && inheritedChangedFromBaseline(path, value, sourceFile)) {
problems.add(NotificationCategory.inheritedChanged);
vc.problemCounter.increment(NotificationCategory.inheritedChanged);
}
VoteStatus voteStatus =
VoteResolver.VoteStatus voteStatus =
userVoteStatus.getStatusForUsersOrganization(sourceFile, path, organization);
boolean itemsOkIfVoted = (voteStatus == VoteStatus.ok);
boolean itemsOkIfVoted = (voteStatus == VoteResolver.VoteStatus.ok);
MissingStatus missingStatus =
onlyRecordErrors
? null
Expand Down Expand Up @@ -695,7 +662,7 @@ private void recordChoice(
}

private void recordLosingDisputedEtc(
String path, VoteStatus voteStatus, MissingStatus missingStatus) {
String path, VoteResolver.VoteStatus voteStatus, MissingStatus missingStatus) {
switch (voteStatus) {
case losing:
if (choices.contains(NotificationCategory.weLost)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import org.unicode.cldr.test.CheckCLDR.Phase;
import org.unicode.cldr.test.CheckWidths;
import org.unicode.cldr.test.DisplayAndInputProcessor;
import org.unicode.cldr.util.VettingViewer.VoteStatus;

/**
* This class implements the vote resolution process agreed to by the CLDR committee. Here is an
Expand Down Expand Up @@ -435,6 +434,40 @@ public boolean canDeleteUsers() {
}
}

/** See VoteResolver getStatusForOrganization to see how this is computed. */
public enum VoteStatus {
/**
* The value for the path is either contributed or approved, and the user's organization
* didn't vote. (see class def for null user)
*/
ok_novotes,

/**
* The value for the path is either contributed or approved, and the user's organization
* chose the winning value. (see class def for null user)
*/
ok,

/**
* The user's organization chose the winning value for the path, but that value is neither
* contributed nor approved. (see class def for null user)
*/
provisionalOrWorse,
Copy link
Member Author

@btangmu btangmu Oct 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment is wrong -- there is no implication whatsoever about the user's organization

This comment will be fixed in the next PR


/**
* The user's organization's choice is not winning. There may be insufficient votes to
* overcome a previously approved value, or other organizations may be voting against it.
* (see class def for null user)
*/
losing,

/**
* There is a dispute, meaning more than one item with votes, or the item with votes didn't
* win.
*/
disputed
}

/** Internal class for voter information. It is public for testing only */
public static class VoterInfo {
private Organization organization;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public String getWinningValueForUsersOrganization(
}

@Override
public VettingViewer.VoteStatus getStatusForUsersOrganization(
public VoteResolver.VoteStatus getStatusForUsersOrganization(
CLDRFile cldrFile, String path, Organization org) {
final CLDRLocale loc = CLDRLocale.getInstance(cldrFile.getLocaleID());
return getVoteResolver(cldrFile, loc, path).getStatusForOrganization(org);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@
import org.unicode.cldr.util.PathHeader.PageId;
import org.unicode.cldr.util.SupplementalDataInfo.PluralInfo.Count;
import org.unicode.cldr.util.VettingViewer.MissingStatus;
import org.unicode.cldr.util.VettingViewer.VoteStatus;
import org.unicode.cldr.util.VoteResolver.Level;
import org.unicode.cldr.util.VoteResolver.Status;
import org.unicode.cldr.util.VoteResolver.VoteStatus;
import org.unicode.cldr.util.VoteResolver.VoterInfo;
import org.unicode.cldr.util.props.ICUPropertyFactory;

Expand Down Expand Up @@ -400,9 +400,9 @@ public void TestVoteStatus() {
resolver.add("fii", toVoterId("appleV"));
VoteStatus voteStatus;
voteStatus = resolver.getStatusForOrganization(Organization.google);
assertEquals("", VoteStatus.ok, voteStatus);
assertEquals("", VoteResolver.VoteStatus.ok, voteStatus);
voteStatus = resolver.getStatusForOrganization(Organization.apple);
assertEquals("", VoteStatus.ok, voteStatus);
assertEquals("", VoteResolver.VoteStatus.ok, voteStatus);

// make non-equal foo
String s1 = "foo";
Expand All @@ -415,7 +415,7 @@ public void TestVoteStatus() {
resolver.setBaseline(s1, Status.approved);
resolver.add(s2, toVoterId("appleV"));
voteStatus = resolver.getStatusForOrganization(Organization.apple);
assertEquals("", VoteStatus.ok, voteStatus);
assertEquals("", VoteResolver.VoteStatus.ok, voteStatus);
}

public void TestLosingStatus() {
Expand All @@ -432,7 +432,7 @@ public void TestLosingStatus() {
resolver.setBaseline("BQ", Status.missing);
resolver.setBaileyValue("bailey");
VoteStatus status = resolver.getStatusForOrganization(Organization.openoffice_org);
assertEquals("", VoteStatus.provisionalOrWorse, status);
assertEquals("", VoteResolver.VoteStatus.provisionalOrWorse, status);

// {lastRelease: {{0}: {1}, missing}, trunk: {null, null}, {orgToVotes:
// pakistan={{0}: {1}=8}, totals: {{0}:
Expand All @@ -444,7 +444,7 @@ public void TestLosingStatus() {
// resolver.setLastRelease("{0}: {1}", Status.missing);
resolver.add("{0}: {1}", toVoterId("adobeE"));
status = resolver.getStatusForOrganization(Organization.openoffice_org);
assertEquals("", VoteStatus.ok, status);
assertEquals("", VoteResolver.VoteStatus.ok, status);

// {lastRelease: {Arabisch, approved}, trunk: {Arabisch, approved},
// {orgToVotes: , totals: {}, conflicted: []},
Expand All @@ -455,7 +455,7 @@ public void TestLosingStatus() {
// resolver.setLastRelease("Arabisch", Status.approved);
resolver.setBaseline("Arabisch", Status.approved);
status = resolver.getStatusForOrganization(Organization.openoffice_org);
assertEquals("", VoteStatus.ok_novotes, status);
assertEquals("", VoteResolver.VoteStatus.ok_novotes, status);
}

public void TestTotalVotesStatus() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import org.unicode.cldr.test.OutdatedPaths;
import org.unicode.cldr.util.PathHeader.PageId;
import org.unicode.cldr.util.PathHeader.SectionId;
import org.unicode.cldr.util.VettingViewer.VoteStatus;
import org.unicode.cldr.util.VoteResolver.VoteStatus;

/** Also see {@link org.unicode.cldr.unittest.TestUtilities} */
class TestVettingViewer {
Expand All @@ -38,7 +38,7 @@ public String getWinningValueForUsersOrganization(
@Override
public VoteStatus getStatusForUsersOrganization(
CLDRFile cldrFile, String path, Organization user) {
return VoteStatus.losing;
return VoteResolver.VoteStatus.losing;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import java.util.Date;
import org.junit.jupiter.api.Test;
import org.unicode.cldr.unittest.TestUtilities;
import org.unicode.cldr.util.VettingViewer.VoteStatus;
import org.unicode.cldr.util.VoteResolver.Status;

/**
Expand Down Expand Up @@ -43,7 +42,8 @@ void testDisputed() {
() -> assertEquals("Illa Bouvet", vr.getWinningValue()),
() ->
assertEquals(
VoteStatus.ok, vr.getStatusForOrganization(Organization.google)));
VoteResolver.VoteStatus.ok,
vr.getStatusForOrganization(Organization.google)));
}

@Test
Expand Down
Loading