Skip to content
Open
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
5 changes: 5 additions & 0 deletions .changeset/sort-scopes-by-service.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@googleworkspace/cli": patch
---

Sort OAuth scopes in the TUI picker by service name first, then access level (read-only before write), then sensitivity classification, for easier browsing
87 changes: 81 additions & 6 deletions src/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,16 @@ pub struct DiscoveredScope {
pub classification: ScopeClassification,
}

/// Compare two scopes for sorting: group by service, then read-only first,
/// then non-sensitive before restricted, then alphabetically.
fn compare_scopes(a: &DiscoveredScope, b: &DiscoveredScope) -> std::cmp::Ordering {
a.api_name
.cmp(&b.api_name)
.then_with(|| b.is_readonly.cmp(&a.is_readonly))
.then_with(|| a.classification.cmp(&b.classification))
.then_with(|| a.short.cmp(&b.short))
}

/// Fetch scopes from discovery docs for the given enabled API IDs.
pub async fn fetch_scopes_for_apis(enabled_api_ids: &[String]) -> Vec<DiscoveredScope> {
let mut all_scopes: Vec<DiscoveredScope> = Vec::new();
Expand Down Expand Up @@ -359,12 +369,10 @@ pub async fn fetch_scopes_for_apis(enabled_api_ids: &[String]) -> Vec<Discovered
}
}

// Sort: restricted first, then sensitive, then non-sensitive, then alphabetically
all_scopes.sort_by(|a, b| {
b.classification
.cmp(&a.classification)
.then_with(|| a.short.cmp(&b.short))
});
// Sort: group by service (api_name), then by access level (read-only first),
// then by classification (non-sensitive before sensitive before restricted),
// then alphabetically by short name.
all_scopes.sort_by(compare_scopes);

all_scopes
}
Expand Down Expand Up @@ -2312,4 +2320,71 @@ mod tests {
assert_eq!(bin, "gcloud");
}
}

#[test]
fn scope_sort_groups_by_service_then_access_then_classification() {
let mut scopes = vec![
DiscoveredScope {
url: "https://www.googleapis.com/auth/gmail".to_string(),
short: "gmail".to_string(),
description: "Full Gmail access".to_string(),
api_name: "Gmail".to_string(),
is_readonly: false,
classification: ScopeClassification::Restricted,
},
DiscoveredScope {
url: "https://www.googleapis.com/auth/drive.readonly".to_string(),
short: "drive.readonly".to_string(),
description: "Read-only Drive".to_string(),
api_name: "Drive".to_string(),
is_readonly: true,
classification: ScopeClassification::NonSensitive,
},
DiscoveredScope {
url: "https://www.googleapis.com/auth/drive".to_string(),
short: "drive".to_string(),
description: "Full Drive access".to_string(),
api_name: "Drive".to_string(),
is_readonly: false,
classification: ScopeClassification::Restricted,
},
DiscoveredScope {
url: "https://www.googleapis.com/auth/drive.metadata".to_string(),
short: "drive.metadata".to_string(),
description: "Drive metadata".to_string(),
api_name: "Drive".to_string(),
is_readonly: false,
classification: ScopeClassification::Sensitive,
},
DiscoveredScope {
url: "https://www.googleapis.com/auth/gmail.readonly".to_string(),
short: "gmail.readonly".to_string(),
description: "Read-only Gmail".to_string(),
api_name: "Gmail".to_string(),
is_readonly: true,
classification: ScopeClassification::NonSensitive,
},
];

scopes.sort_by(compare_scopes);

// Assert the full sort order of scope short names.
// Order: by service (alpha), then read-only before write,
// then by classification (Sensitive before Restricted), then by short name.
let sorted_shorts: Vec<_> = scopes.iter().map(|s| s.short.as_str()).collect();
assert_eq!(
sorted_shorts,
&[
"drive.readonly",
"drive.metadata",
"drive",
"gmail.readonly",
"gmail",
]
);

// Verify classification sorting for the Drive write scopes.
assert_eq!(scopes[1].classification, ScopeClassification::Sensitive);
assert_eq!(scopes[2].classification, ScopeClassification::Restricted);
}
Comment on lines +2325 to +2389
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The test plan mentions verifying the sort order by service, access level, and classification, but this test case only effectively validates the first two criteria (api_name and is_readonly). The current test data doesn't include items that would have the same api_name and is_readonly status but different classification or short names, so the third and fourth levels of the sort logic are not being exercised.

To make the test more robust and fully cover the new sorting logic, please consider adding more scopes to the test vector. For example, you could add two scopes with the same api_name and is_readonly value but different classifications to verify that classification sorting works as expected.

}
Loading