[BB-1927] remove business+ dependency for getting workspace role grants

[aldevv]

• removed business+ dependency for processing workspace roles grants
  currently not possible to get system roles because the admin.roles.read permission is required to access the listAssignments endpoint, since business+ only has access to admin permission, not admin.roles.read, then is not possible to list those.

Summary by CodeRabbit

• Refactor
  • Simplified workspace grants retrieval by removing page-based pagination and rate-limiting handling. Sync operations now return results without paging metadata.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Changes to pkg/connector/workspace.go simplify the Grants function by removing Business+ client integration and page-based pagination. Workspace grants now fetch users directly via GetUsers() without paging metadata, returning immediately on error. Annotation and rate-limit handling have been eliminated.

Changes

Cohort / File(s)	Summary
Workspace Grants Simplification pkg/connector/workspace.go	Removed Business+ client integration and page-based pagination from Grants. Eliminated page token parsing, paging metadata collection, rate-limit annotations, and context-based logging (ctxzap). Grants now return flat user set via direct GetUsers() call with empty SyncOpResults.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• [BB-1925] Add workspace role #69 — Modifies Grants in the same file and adjusts Business+ client usage patterns for workspace membership and role handling.

Suggested reviewers

• btipling
• luisina-santos

Poem

🐰 Pagination gone, pagination done,
No Business+ dance beneath the sun,
Simple grants now flow so free,
Direct and swift, as grants should be! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly summarizes the main change: removing the Business+ dependency for workspace role grants, which aligns with the core objective.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b44d836 and 6e81094.

📒 Files selected for processing (1)

• pkg/connector/workspace.go

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-10-28T17:48:53.644Z

Learnt from: mateoHernandez123
Repo: ConductorOne/baton-slack PR: 57
File: pkg/connector/roles.go:217-219
Timestamp: 2025-10-28T17:48:53.644Z
Learning: In the baton-slack connector, when client methods like SetWorkspaceRole(), RemoveUser(), or other enterprise/slack client calls already wrap errors with specific gRPC codes via handleError(), use fmt.Errorf with %w to preserve those codes while adding context. Only use uhttp.WrapErrors for errors generated directly in the function (e.g., validation, precondition checks). Double-wrapping with uhttp.WrapErrors would overwrite the specific codes (Unavailable, PermissionDenied, etc.) needed for proper retry logic and alerting.
<!-- [/add_learning]

Applied to files:

• pkg/connector/workspace.go

🧬 Code graph analysis (1)

pkg/connector/workspace.go (1)

pkg/connector/client/helpers.go (1)

• WrapError (41-74)

🔇 Additional comments (2)

pkg/connector/workspace.go (2)

21-21: businessPlusClient field is still actively used and should not be removed.

The businessPlusClient is actively used throughout the codebase for SCIM API operations including:

• User listing (user.go)
• IDP group listing and management (group.go)
• User enable/disable actions (actions.go)

Removing it from the Grants method does not make the field itself unnecessary.

Likely an incorrect or invalid review comment.

---

141-144: Fix compilation error: GetUsers() requires context and options parameters.

Line 141 calls o.client.GetUsers() with no arguments, but the slack-go library's GetUsers() method requires context and options. The correct call pattern is GetUsersContext(ctx, options) as shown in user.go.

Additionally, this change removes pagination support. The Slack API's users.list endpoint returns paginated results, and large workspaces may not receive all users in a single call without handling the pagination cursor.

Replace the call with:

options := slack.GetUsersOptionTeamID(resource.Id)
users, err := o.client.GetUsersContext(ctx, options)

This preserves pagination handling and matches the pattern used in user.go.

⛔ Skipped due to learnings

Learnt from: mateoHernandez123
Repo: ConductorOne/baton-slack PR: 57
File: pkg/connector/roles.go:217-219
Timestamp: 2025-10-28T17:48:53.644Z
Learning: In the baton-slack connector, when client methods like SetWorkspaceRole(), RemoveUser(), or other enterprise/slack client calls already wrap errors with specific gRPC codes via handleError(), use fmt.Errorf with %w to preserve those codes while adding context. Only use uhttp.WrapErrors for errors generated directly in the function (e.g., validation, precondition checks). Double-wrapping with uhttp.WrapErrors would overwrite the specific codes (Unavailable, PermissionDenied, etc.) needed for proper retry logic and alerting.
<!-- [/add_learning]

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[laurenleach]

I think we could emit the grants from the users Grants method to avoid listing users here. We should have the workspace id since it is the parent resource