Add configurable pass threshold base

docs/config/.gitvote.yml

@@ -83,10 +83,56 @@ profiles:
     # Percentage of votes in favor required to pass the vote
     #
     # The percentage is calculated based on the number of votes in favor and the
-    # number of allowed voters (see allowed_voters field below for more details).
+    # pass threshold base, which is the number of allowed voters by default (see
+    # the pass_threshold_base and allowed_voters fields below for more details).
     # It must be greater than 0 and not greater than 100.
     pass_threshold: 50
 
+    # Pass threshold base (optional)
+    #
+    # Number the in favor and against percentages are calculated over, which is
+    # also used to decide if the vote passes. Only binding votes are counted.
+    #
+    #   - allowed_voters (default): all users with binding votes, so users who
+    #     do not vote count towards the percentage calculation too.
+    #
+    #   - votes_cast: binding votes cast (in favor, against and abstain).
+    #     Abstentions are included in the percentage calculation by default.
+    #     When `exclude_abstentions` is set to true, only votes in favor and
+    #     against are included. Abstentions are still displayed and recorded
+    #     as binding votes, though.
+    #
+    # If no votes are counted, the vote does not pass. Please note that there is
+    # no minimum participation required when using votes_cast, so a low turnout
+    # can decide a vote.
+    #
+    # Each profile sets this option independently (the value set in the default
+    # profile is not inherited by other profiles).
+    #
+    # pass_threshold_base: allowed_voters
+    #
+    # pass_threshold_base:
+    #   votes_cast: {}
+    #
+    # pass_threshold_base:
+    #   votes_cast:
+    #     exclude_abstentions: true
+    #
+    # More votes in favor than against
+    #
+    # To require more votes in favor than against (ignoring abstentions and users
+    # who do not vote), use votes_cast excluding abstentions with a pass
+    # threshold of 50.01. A tie does not pass with 50.01, but it would pass with
+    # 50. This is exact for up to 2,499 votes against. Comments will display
+    # `50.01%` as the passing threshold.
+    #
+    # pass_threshold: 50.01
+    # pass_threshold_base:
+    #   votes_cast:
+    #     exclude_abstentions: true
+    #
+    pass_threshold_base: allowed_voters
+
     # Allowed voters (optional)
     #
     # List of GitHub teams and users who have binding votes
@@ -156,6 +202,12 @@ profiles:
     # The close on passing feature makes this possible. Open votes where this
     # feature has been enabled will be checked once daily and, if GitVote
     # detects that the vote has passed, it will automatically close it.
+    #
+    # Whatever the pass_threshold_base is set to, a vote is only closed early
+    # once the pass threshold is met over all allowed voters, so that votes from
+    # the users who have not voted yet cannot make it fail. When using
+    # votes_cast, this means that a vote that is currently passing may stay open
+    # until its duration ends.
     # 
     # close_on_passing: true
     #

src/cfg_repo.rs

@@ -98,6 +98,14 @@ pub(crate) struct CfgProfile {
     pub allowed_voters: Option<AllowedVoters>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub announcements: Option<Announcements>,
+    // The singleton map representation allows using the `votes_cast:` map syntax
+    // in YAML, instead of the `!votes_cast` tag expected by default
+    #[serde(
+        default,
+        skip_serializing_if = "Option::is_none",
+        with = "serde_yaml::with::singleton_map"
+    )]
+    pub pass_threshold_base: Option<PassThresholdBase>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub periodic_status_check: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
@@ -174,6 +182,20 @@ pub(crate) struct DiscussionsAnnouncements {
     pub category: String,
 }
 
+/// Base used to calculate the pass threshold percentage.
+#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)]
+#[serde(rename_all = "snake_case")]
+pub(crate) enum PassThresholdBase {
+    /// All users with binding votes.
+    #[default]
+    AllowedVoters,
+    /// Binding votes cast.
+    VotesCast {
+        #[serde(skip_serializing_if = "Option::is_none")]
+        exclude_abstentions: Option<bool>,
+    },
+}
+
 /// Errors that may occur while getting the configuration profile.
 #[derive(Debug, Error, Clone, PartialEq, Serialize, Deserialize)]
 pub(crate) enum CfgError {

src/cfg_repo/tests.rs

@@ -149,6 +149,69 @@ fn automation_rule_matches_anchored_pattern_only_from_root() {
     );
 }
 
+#[test]
+fn cfg_profile_deserialize_pass_threshold_base() {
+    // Setup cases (YAML snippet, expected pass threshold base)
+    let cases = [
+        ("", None),
+        (
+            "pass_threshold_base: allowed_voters",
+            Some(PassThresholdBase::AllowedVoters),
+        ),
+        (
+            "pass_threshold_base:\n  votes_cast: {}",
+            Some(PassThresholdBase::VotesCast {
+                exclude_abstentions: None,
+            }),
+        ),
+        (
+            "pass_threshold_base:\n  votes_cast:",
+            Some(PassThresholdBase::VotesCast {
+                exclude_abstentions: None,
+            }),
+        ),
+        (
+            "pass_threshold_base:\n  votes_cast:\n    exclude_abstentions: true",
+            Some(PassThresholdBase::VotesCast {
+                exclude_abstentions: Some(true),
+            }),
+        ),
+        (
+            "pass_threshold_base:\n  votes_cast:\n    exclude_abstentions: false",
+            Some(PassThresholdBase::VotesCast {
+                exclude_abstentions: Some(false),
+            }),
+        ),
+    ];
+
+    for (snippet, expected) in cases {
+        // Check the profile parses with the expected pass threshold base
+        let yaml = format!("duration: 5m\npass_threshold: 50\n{snippet}\n");
+        let cfg: CfgProfile = serde_yaml::from_str(&yaml).unwrap_or_else(|err| panic!("{snippet}: {err}"));
+        assert_eq!(cfg.pass_threshold_base, expected, "snippet: {snippet}");
+    }
+}
+
+#[test]
+fn cfg_profile_deserialize_pass_threshold_base_invalid() {
+    // Setup invalid YAML snippets
+    let cases = [
+        "pass_threshold_base: other",
+        "pass_threshold_base: votes_cast",
+        "pass_threshold_base:\n  allowed_voters: {}",
+        "pass_threshold_base:\n  votes_cast:\n    exclude_abstentions: \"yes\"",
+    ];
+
+    for snippet in cases {
+        // Check the profile is rejected
+        let yaml = format!("duration: 5m\npass_threshold: 50\n{snippet}\n");
+        assert!(
+            serde_yaml::from_str::<CfgProfile>(&yaml).is_err(),
+            "snippet: {snippet}"
+        );
+    }
+}
+
 #[test]
 fn cfg_profile_deserialize_stored_json() {
     // Setup JSON as stored in the vote cfg column
@@ -187,6 +250,7 @@ fn cfg_profile_deserialize_stored_json() {
                     category: DISCUSSIONS_CATEGORY.to_string(),
                 }),
             }),
+            pass_threshold_base: None,
             periodic_status_check: Some("1 week".to_string()),
             close_on_passing: Some(true),
             close_on_passing_min_wait: Some("2 days".to_string()),
@@ -214,6 +278,53 @@ fn cfg_profile_deserialize_stored_json_minimal() {
     );
 }
 
+#[test]
+fn cfg_profile_deserialize_stored_json_pass_threshold_base() {
+    // Setup cases (stored JSON value, expected pass threshold base)
+    let cases = [
+        (json!("allowed_voters"), PassThresholdBase::AllowedVoters),
+        (
+            json!({"votes_cast": {}}),
+            PassThresholdBase::VotesCast {
+                exclude_abstentions: None,
+            },
+        ),
+        (
+            json!({"votes_cast": {"exclude_abstentions": true}}),
+            PassThresholdBase::VotesCast {
+                exclude_abstentions: Some(true),
+            },
+        ),
+    ];
+
+    for (stored_base, expected) in cases {
+        // Check the stored profile deserializes into the expected one
+        let stored = json!({
+            "duration": "5m",
+            "pass_threshold": 50.0,
+            "pass_threshold_base": stored_base.clone()
+        });
+        let cfg: CfgProfile = serde_json::from_value(stored.clone()).unwrap();
+        assert_eq!(
+            cfg,
+            CfgProfile {
+                duration: Duration::from_mins(5),
+                pass_threshold: 50.0,
+                pass_threshold_base: Some(expected),
+                ..Default::default()
+            },
+            "stored: {stored_base}"
+        );
+
+        // Check it serializes back into the same JSON
+        assert_eq!(
+            serde_json::to_value(&cfg).unwrap(),
+            stored,
+            "stored: {stored_base}"
+        );
+    }
+}
+
 #[test]
 fn cfg_profile_serialize_full() {
     // Setup profile with all fields set
@@ -230,6 +341,9 @@ fn cfg_profile_serialize_full() {
                 category: DISCUSSIONS_CATEGORY.to_string(),
             }),
         }),
+        pass_threshold_base: Some(PassThresholdBase::VotesCast {
+            exclude_abstentions: Some(true),
+        }),
         periodic_status_check: Some("1 day".to_string()),
         close_on_passing: Some(true),
         close_on_passing_min_wait: Some("1 hour".to_string()),
@@ -251,6 +365,11 @@ fn cfg_profile_serialize_full() {
                     "category": DISCUSSIONS_CATEGORY
                 }
             },
+            "pass_threshold_base": {
+                "votes_cast": {
+                    "exclude_abstentions": true
+                }
+            },
             "periodic_status_check": "1 day",
             "close_on_passing": true,
             "close_on_passing_min_wait": "1 hour"
@@ -407,6 +526,9 @@ profiles:
     announcements:
       discussions:
         category: announcements
+    pass_threshold_base:
+      votes_cast:
+        exclude_abstentions: true
     periodic_status_check: 1 week
     close_on_passing: true
     close_on_passing_min_wait: 1 day
@@ -433,6 +555,9 @@ profiles:
                             category: DISCUSSIONS_CATEGORY.to_string(),
                         }),
                     }),
+                    pass_threshold_base: Some(PassThresholdBase::VotesCast {
+                        exclude_abstentions: Some(true),
+                    }),
                     periodic_status_check: Some("1 week".to_string()),
                     close_on_passing: Some(true),
                     close_on_passing_min_wait: Some("1 day".to_string()),
@@ -531,6 +656,32 @@ async fn get_cfg_profile_invalid_config_invalid_yaml() {
     ));
 }
 
+#[tokio::test]
+async fn get_cfg_profile_invalid_config_pass_threshold_base() {
+    // Setup GitHub expectations
+    let mut gh = MockGH::new();
+    gh.expect_get_config_file()
+        .with(eq(INST_ID), eq(OWNER), eq(REPO))
+        .times(1)
+        .returning(|_, _, _| {
+            let config = r"
+profiles:
+  default:
+    duration: 5m
+    pass_threshold: 50
+    pass_threshold_base: other
+";
+            Box::pin(future::ready(Some(config.to_string())))
+        });
+    let gh = Arc::new(gh);
+
+    // Run and check the error returned
+    assert!(matches!(
+        CfgProfile::get(gh, INST_ID, OWNER, OWNER_IS_ORG, REPO, None).await.unwrap_err(),
+        CfgError::InvalidConfig(_)
+    ));
+}
+
 #[tokio::test]
 async fn get_cfg_profile_invalid_config_teams_owner_not_org() {
     // Setup GitHub expectations
@@ -557,6 +708,41 @@ async fn get_cfg_profile_invalid_config_teams_owner_not_org() {
     );
 }
 
+#[tokio::test]
+async fn get_cfg_profile_pass_threshold_base_votes_cast() {
+    // Setup GitHub expectations
+    let mut gh = MockGH::new();
+    gh.expect_get_config_file()
+        .with(eq(INST_ID), eq(OWNER), eq(REPO))
+        .times(1)
+        .returning(|_, _, _| {
+            let config = r"
+profiles:
+  default:
+    duration: 5m
+    pass_threshold: 50.01
+    pass_threshold_base:
+      votes_cast:
+        exclude_abstentions: true
+";
+            Box::pin(future::ready(Some(config.to_string())))
+        });
+    let gh = Arc::new(gh);
+
+    // Run and check the profile returned
+    assert_eq!(
+        CfgProfile::get(gh, INST_ID, OWNER, OWNER_IS_ORG, REPO, None).await.unwrap(),
+        CfgProfile {
+            duration: Duration::from_mins(5),
+            pass_threshold: 50.01,
+            pass_threshold_base: Some(PassThresholdBase::VotesCast {
+                exclude_abstentions: Some(true),
+            }),
+            ..Default::default()
+        }
+    );
+}
+
 #[tokio::test]
 async fn get_cfg_profile_profile1() {
     // Setup GitHub expectations

src/processor.rs

@@ -519,8 +519,9 @@ impl StatusChecker {
     }
 }
 
-/// Worker that periodically auto closes votes that have already passed. This
-/// only applies to votes with the `close_on_passing` feature enabled.
+/// Worker that periodically auto closes votes that have already passed over all
+/// the allowed voters. This only applies to votes with the `close_on_passing`
+/// feature enabled.
 struct VotesAutoCloser {
     db: DynDB,
     gh: DynGH,
@@ -546,8 +547,12 @@ impl VotesAutoCloser {
                             // Calculate vote results
                             match results::calculate(self.gh.clone(), owner, repo, vote).await {
                                 Ok(results) => {
-                                    // If the vote has already passed, update its ending timestamp
-                                    if results.passed
+                                    // Close the vote early (update its ending
+                                    // timestamp) only once it passes over all the
+                                    // allowed voters, so that further votes from
+                                    // pending voters cannot make it fail. A vote
+                                    // passing over the votes cast may stay open.
+                                    if results.passes_with_all_allowed_voters()
                                         && let Err(err) = self.db.update_vote_ends_at(vote_id).await
                                     {
                                         error!(?err, ?vote.vote_id, "error updating vote ends at");