Skip to content

Conversation

Copy link

Copilot AI commented Dec 6, 2025

Details

Addresses multiple review suggestions from #18589 to improve code quality, consistency, and maintainability across the FleetDM connector implementation.

Changes to FleetConnectorForm.ts:

  • Added public override modifier to getSuccessMessage() method for proper method override declaration
  • Capitalized "Fleet" in success messages for consistency with connector naming
  • Replaced ifDefined() directive with nullish coalescing operator (??) for cleaner, more idiomatic TypeScript
  • Replaced verbose manual switch HTML with ak-switch-input component for consistency with codebase patterns
  • Removed unused ifDefined import

Changes to FleetConnectorViewPage.ts:

  • Added public override modifier to updated() lifecycle method for consistency with willUpdate() method
  • Removed PFBase import and from styles array (already provided by AKElement base class)
  • Added protected access modifier to renderTabOverview() method
  • Moved ModelEnum split operation to module-level constant for better performance and reusability
  • Fixed aria-label formatting to use template literal syntax consistently

These changes align with established patterns throughout the web codebase (e.g., SearchSelect.ts, HorizontalFormElement.ts) and address all 8 review comments from the original PR.


Checklist

  • Local tests pass (ak test authentik/)
  • The code has been formatted (make lint-fix)

If an API change has been made

  • The API schema has been updated (make gen-build)

If changes to the frontend have been made

  • The code has been formatted (make web)

If applicable

  • The documentation has been updated
  • The documentation has been formatted (make docs)

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@netlify
Copy link

netlify bot commented Dec 6, 2025

Deploy Preview for authentik-docs canceled.

Name Link
🔨 Latest commit 4a33e99
🔍 Latest deploy log https://app.netlify.com/projects/authentik-docs/deploys/6934a1d5ecdd7b000889b023

@netlify
Copy link

netlify bot commented Dec 6, 2025

Deploy Preview for authentik-integrations canceled.

Name Link
🔨 Latest commit 4a33e99
🔍 Latest deploy log https://app.netlify.com/projects/authentik-integrations/deploys/6934a1d5646d170008b6e9a6

Copilot AI changed the title [WIP] WIP address feedback on FleetDM connector PR Add public override modifiers to updated method in FleetConnectorViewPage Dec 6, 2025
Copilot AI requested a review from GirlBossRush December 6, 2025 20:01
@GirlBossRush
Copy link
Contributor

@copilot This is a nice small improvement but please include some more of the suggested changes in #18589

Copy link
Author

Copilot AI commented Dec 6, 2025

@copilot This is a nice small improvement but please include some more of the suggested changes in #18589

I've addressed all 8 review suggestions from #18589 in commits d16dcbc and 4a33e99:

FleetConnectorForm.ts:

  • Added public override to getSuccessMessage()
  • Capitalized "Fleet" in success messages
  • Replaced ifDefined() with nullish coalescing (??)
  • Replaced manual switch with ak-switch-input component

FleetConnectorViewPage.ts:

  • Added public override to updated() method (already done in bfca40c)
  • Removed PFBase from styles array
  • Added protected to renderTabOverview()
  • Moved ModelEnum split to constant
  • Fixed aria-label formatting

Copilot AI changed the title Add public override modifiers to updated method in FleetConnectorViewPage Address review feedback on FleetDM connector implementation Dec 6, 2025
@GirlBossRush GirlBossRush marked this pull request as ready for review December 7, 2025 18:42
@GirlBossRush GirlBossRush requested a review from a team as a code owner December 7, 2025 18:42
@GirlBossRush GirlBossRush merged commit 4487439 into endpoints/fleet-connector Dec 8, 2025
10 checks passed
@GirlBossRush GirlBossRush deleted the copilot/sub-pr-18589 branch December 8, 2025 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants