Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 22, 2026

Description

Addresses code review feedback from #756 focusing on documentation clarity, code quality, and a memory leak in stream disposal.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Changes

Documentation

  • Added XML documentation for zero ID filtering behavior in MergeTargetUserIds and BuildTargetUsersCsvFile
  • Clarified CSV format requirements in UpdateInviteTargetUsersAsync (header line must be "Users", one ID per subsequent line)
  • Documented silent skipping of malformed CSV lines in ParseTargetUsersCsv
  • Added stream validation (CanRead check) in CreateInviteAsync with exception documentation

Bug Fixes

  • Fixed memory leak: changed resetPositionTo: 0 to resetPositionTo: null for internally-created MemoryStreams in BuildTargetUsersCsvFile
    • Previous behavior prevented HTTP client disposal by signaling stream reuse
    • New behavior allows proper disposal after multipart request completes
  • Removed misleading parameter name from exception in UpdateInviteTargetUsersAsync (could be any of 3 parameters)

Code Quality

  • Refactored to use explicit LINQ filtering (Where(), Select()) instead of implicit filtering in loops
  • Applied to: BuildTargetUsersCsvFile, MergeTargetUserIds, ParseTargetUsersCsv

How Has This Been Tested?

  • Build verification across all target frameworks (net8.0, net9.0, net10.0)
  • Automated code review (no issues)

Test Configuration:

  • SDK: .NET 8.0, 9.0, 10.0

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

@Aiko-IT-Systems/discatsharp


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits January 22, 2026 03:43
…ream

Co-Authored-By: copilot-swe-agent[bot] <[email protected]>

Co-authored-by: Lulalaby <[email protected]>
Copilot AI changed the title [WIP] Add support for invite target user allowlists and job status refactor: address code review feedback on invite target users feature Jan 22, 2026
Copilot AI requested a review from Lulalaby January 22, 2026 03:52
@Lulalaby Lulalaby marked this pull request as ready for review January 22, 2026 03:52
@Lulalaby Lulalaby requested a review from a team as a code owner January 22, 2026 03:52
@Lulalaby Lulalaby requested review from cking and xMaxximum and removed request for a team January 22, 2026 03:52
@Lulalaby Lulalaby merged commit a7baa78 into feat/invites-update Jan 22, 2026
4 checks passed
@Lulalaby Lulalaby deleted the copilot/sub-pr-756 branch January 22, 2026 03:53
Lulalaby added a commit that referenced this pull request Jan 22, 2026
…756)

* feat: add support for invite target user allowlists and job status

Introduces methods and endpoints to manage allowlists of users who can accept invites, including CSV-based and user ID-based updates. Adds the ability to query the processing status of invite target user jobs, updates the invite creation API to support role grants and target user restrictions, and implements supporting types and utilities for these features.

* Rename Values to SelectedValues in checkbox group

Renamed the 'Values' property to 'SelectedValues' in DiscordCheckboxGroupComponent to improve clarity and better reflect its purpose.

* refactor: address code review feedback on invite target users feature (#757)

* Initial plan

* docs: improve documentation and code quality for invite target users

Co-Authored-By: copilot-swe-agent[bot] <[email protected]>

Co-authored-by: Lulalaby <[email protected]>

* fix: prevent memory leak by not resetting internally-created MemoryStream

Co-Authored-By: copilot-swe-agent[bot] <[email protected]>

Co-authored-by: Lulalaby <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: Lulalaby <[email protected]>

* Dispose targetUsersCsv streams after use

Added try-catch blocks to dispose the targetUsersCsv streams in CreateChannelInviteAsync and UpdateInviteTargetUsersAsync methods to ensure proper resource cleanup and prevent potential memory leaks.

---------

Co-authored-by: Copilot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants