Skip to content

Test PR - Ignore #3801

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 50 commits into from
Closed

Test PR - Ignore #3801

wants to merge 50 commits into from

Conversation

daveverwer
Copy link
Member

@daveverwer daveverwer commented May 19, 2025

Summary by CodeRabbit

  • New Features

    • Introduced a user portal with authentication features, including login, signup, password reset, email verification, logout, and account deletion.
    • Added dedicated portal pages and forms for user management actions.
    • Updated navigation menus to include a portal link (non-production environments).
    • Enhanced navigation and form styling for portal-related pages.
    • Improved dark mode image support and added new icons.
  • Bug Fixes

    • Adjusted navigation alignment and search bar width for improved layout consistency.
  • Tests

    • Added comprehensive automated tests covering portal authentication flows, error handling, and user management scenarios.
  • Chores

    • Enabled secure session management and cookies.
    • Updated documentation and test snapshots to reflect new portal features.

@cla-bot cla-bot bot added the cla-signed label May 19, 2025
Copy link

coderabbitai bot commented May 19, 2025

📝 Walkthrough

Walkthrough

This change introduces a comprehensive authentication and user management system within the application, primarily through the addition of a "portal" feature. New controllers, views, and routes are implemented for login, signup, logout, password reset, account verification, and account deletion, leveraging AWS Cognito for authentication. The navigation menu is updated to conditionally display a "Portal" link based on the environment. Session management is enabled and configured for secure cookies. The frontend receives new and updated SCSS files for portal-related styling and navigation adjustments. Extensive tests are added to validate portal flows and error handling, with dependency injection used for Cognito operations.

Note

⚡️ AI Code Reviews for VS Code, Cursor, Windsurf

CodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback.
Learn more here.


Note

⚡️ Faster reviews with caching

CodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure Review - Disable Cache at either the organization or repository level. If you prefer to disable all data retention across your organization, simply turn off the Data Retention setting under your Organization Settings.
Enjoy the performance boost—your workflow just got faster.


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 28

🧹 Nitpick comments (20)
Sources/App/Controllers/Portal/DeleteAccountController.swift (1)

14-24: Consider enhancing error logging

The current error handling includes the error's localizedDescription in the user-facing message, but consider adding more detailed logging for better troubleshooting capability.

 static func deleteAccount(req: Request) async throws -> Response {
     @Dependency(\.cognito) var cognito
     do {
         try await cognito.deleteUser(req: req)
         req.auth.logout(AuthenticatedUser.self)
         req.session.unauthenticate(AuthenticatedUser.self)
         req.session.destroy()
         return req.redirect(to: SiteURL.home.relativeURL())
     } catch {
+        req.logger.error("Failed to delete user account: \(error)")
         return PortalPage.View(path: SiteURL.portal.relativeURL(), model: PortalPage.Model(errorMessage: "An unknown error occurred: \(error.localizedDescription)")).document().encodeResponse(status: .internalServerError)
     }
 }
FrontEnd/styles/portal.scss (1)

19-29: Styling for portal forms

The CSS classes provide a good baseline for form layouts. Consider adding responsive design adjustments for smaller screens, as the current fixed width percentage might not work well on mobile devices.

 .portal-form-container {
     height: 55vh;
     padding: 10%;
 }

 .portal-form-inputs {
     display: flex;
     flex-direction: column;
     width: 50%;
     margin-bottom: 15px;
+    
+    @media (max-width: 768px) {
+        width: 100%;
+    }
 }
Sources/App/Views/NavMenuItems.swift (1)

69-80: Consider adding text alongside the portal icon.

The implementation looks correct, but unlike other menu items (except .search), this case uses only an image without accompanying text. This could be a potential accessibility concern for screen readers.

Consider adding text after the image for better accessibility:

            case .portal:
                return .li(
                    .class("portal"),
                    .a(
                        .href(SiteURL.portal.relativeURL()),
                        .img(
                            .alt("Portal"),
                            .src(SiteURL.images("portal.svg").relativeURL()),
                            .width(20)
-                        )
+                        ),
+                        " Portal"
                    )
                )
Sources/App/Controllers/Portal/LoginController.swift (2)

1-9: Missing Apache 2.0 license header

Per repository guidelines every Swift file must start with the standard Apache 2.0 preamble. Please add it here.


47-50: Return path should match failing request

When an AWS client / generic error occurs you redirect to SiteURL.login.relativeURL() instead of using req.url.path, which you use for Cognito errors. This causes the action attribute of the rendered form to differ depending on the failure branch. For consistency (and easier testing) use the current request path in all branches.

Sources/App/Core/SiteURL.swift (1)

185-187: delete path may collide with other DELETE endpoints

case .deleteAccount: return "delete" introduces a top-level /delete route.
Consider a more explicit slug ("delete-account" or "account/delete") to avoid accidental overlap with future RESTful delete handlers.

Sources/App/Controllers/Portal/VerifyController.swift (1)

1-2: Missing Apache 2.0 license header

Add the standard license block to comply with the repository’s Swift file guideline.

Sources/App/Views/Portal/ForgotPassword+View.swift (1)

40-40: Fix extra whitespace.

There's unnecessary whitespace after the emailField(email: email) method call.

-            .emailField(email: email)   ,
+            .emailField(email: email),
Sources/App/Views/Portal/Successful+Password+Change.swift (2)

26-26: Consider using a heading tag for the success message.

For consistency with other portal views like Login, Signup, and ForgotPassword that use h2 tags for their headings, consider wrapping the success message in an h2 tag.

            .div(
                .class("portal-form-container"),
-                .text(self.model.successMessage),
+                .h2(self.model.successMessage),
                .loginRedirectButton()
            )

36-36: Add method attribute to form for consistency.

All other forms in the portal views explicitly specify a method (usually POST). For consistency, consider adding a method attribute to this form as well.

        .form(
            .action(SiteURL.login.relativeURL()),
+            .method(.post),
            .button(
Sources/App/Controllers/Portal/ForgotPasswordController.swift (2)

18-18: Remove extra whitespace in return type.

There's an extra space after the arrow in the function return type.

-        static func forgotPasswordEmail(req: Request) async throws ->  HTML {
+        static func forgotPasswordEmail(req: Request) async throws -> HTML {

24-26: Consider validating the email format.

The code extracts the email from the request but doesn't validate its format before sending it to Cognito. Adding validation could improve the user experience by providing immediate feedback.

             do {
                 let user = try req.content.decode(Credentials.self)
+                guard user.email.contains("@") && user.email.contains(".") else {
+                    return ForgotPassword.View(path: req.url.path, model: ForgotPassword.Model(errorMessage: "Please enter a valid email address")).document()
+                }
                 try await cognito.forgotPassword(req: req, username: user.email)
                 return Reset.View(path: SiteURL.resetPassword.relativeURL(), model: Reset.Model(email: user.email)).document()
Sources/App/Views/Portal/Login+View.swift (2)

6-8: Model does not capture user-entered data.

If a login attempt fails, you currently cannot repopulate the email field because the Model only stores an errorMessage. Consider extending the model to hold email (and optionally password) so the form can be redisplayed with the previously entered value.


24-35: Pass model data into the form for better UX.

loginForm() is called without parameters, so the email/password fields are always empty on rerender. After augmenting Model, pass the values through:

-                .loginForm(),
+                .loginForm(email: model.email),
Sources/App/Views/Portal/Reset+View.swift (1)

25-28: Pre-populate form fields with the model’s values.

The Reset.Model already contains email; pass it (and the code if appropriate) so the user doesn’t need to retype after validation errors.

-                .resetPasswordForm(),
+                .resetPasswordForm(email: model.email),
Sources/App/Core/Cognito.swift (2)

29-47: Remove redundant Void return type.

The Void return type in the function declaration is redundant and could be removed.

-    static func authenticateToken(req: Request, sessionID: String, accessToken: String) async throws -> Void {
+    static func authenticateToken(req: Request, sessionID: String, accessToken: String) async throws {
🧰 Tools
🪛 SwiftLint (0.57.0)

[Warning] 30-30: Returning Void in a function declaration is redundant

(redundant_void_return)


49-148: Consider refactoring to reduce code duplication.

There's significant duplication in the setup of the AWS client and Cognito configuration across all methods. This could be refactored into a helper method to improve maintainability.

Consider implementing a helper method like this:

private static func configureCognito(for req: Request) -> (AWSClient, CognitoConfiguration) {
    let awsClient = AWSClient(httpClientProvider: .shared(req.application.http.client.shared))
    let awsCognitoConfiguration = CognitoConfiguration(
        userPoolId: Environment.get("AWS_COGNITO_POOL_ID")!,
        clientId: Environment.get("AWS_COGNITO_CLIENT_ID")!,
        clientSecret: Environment.get("AWS_COGNITO_CLIENT_SECRET")!,
        cognitoIDP: CognitoIdentityProvider(client: awsClient, region: .useast2),
        adminClient: true
    )
    return (awsClient, awsCognitoConfiguration)
}

Then each method could use it like:

@Sendable
static func method(req: Request, /* other params */) async throws {
    let (awsClient, configuration) = configureCognito(for: req)
    do {
        req.application.cognito.authenticatable = CognitoAuthenticatable(configuration: configuration)
        // Method-specific code
        try await awsClient.shutdown()
    } catch {
        try await awsClient.shutdown()
        throw error
    }
}
FrontEnd/styles/images.scss (1)

20-24: Consider moving large Base-64 sprites to separate asset files

Embedding sizeable Base-64 blobs directly in the stylesheet inflates the CSS payload shipped to every visitor (the data is duplicated in both light and dark sections and cannot be cached independently). For rarely-changed images, shipping them as separate static assets (and referencing them with a relative URL) typically yields:

• smaller CSS transfer size
• better browser caching characteristics
• clearer diffs when artwork is updated

If inlined delivery is required (e.g. for critical-path icons), consider extracting the common definitions to a single mixin/partial to avoid duplicating the same 1.8 KB of data between the two theme blocks.

Also applies to: 27-28

Tests/AppTests/PortalTests.swift (2)

38-50: Avoid optional-data force-unwraps in unit tests

Swift-lint rightly flags String(...).data(using:.utf8)!.
You can drop the optional entirely with the non-optional initialiser:

-let jsonData: Data = """
-{
-   ...
-}
-""".data(using: .utf8)!
+let jsonData = Data("""
+{
+   ...
+}
+""".utf8)

Safer, shorter, and silences the linter.

Also applies to: 66-78

🧰 Tools
🪛 SwiftLint (0.57.0)

[Warning] 39-39: Prefer non-optional Data(_:) initializer when converting String to Data

(non_optional_string_data_conversion)


297-313: Verify session-cookie assertions

res.headers.setCookie?["vapor-session"] returns HTTPCookies.Value?, not
String?, so the equality check will always be false unless casted.
Use if case .none = res.headers.setCookie?["vapor-session"] { … }
or compare against nil after optional-chaining appropriately.

Minor, but prevents false positives.

Also applies to: 385-395

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between b5b5cca and a30defa.

⛔ Files ignored due to path filters (3)
  • Public/images/portal.svg is excluded by !**/*.svg
  • Resources/SVGs/account~dark.svg is excluded by !**/*.svg
  • Resources/SVGs/account~light.svg is excluded by !**/*.svg
📒 Files selected for processing (35)
  • FrontEnd/main.scss (1 hunks)
  • FrontEnd/styles/header_footer.scss (2 hunks)
  • FrontEnd/styles/images.scss (2 hunks)
  • FrontEnd/styles/portal.scss (1 hunks)
  • FrontEnd/styles/search.scss (1 hunks)
  • Sources/App/Controllers/Portal/DeleteAccountController.swift (1 hunks)
  • Sources/App/Controllers/Portal/ForgotPasswordController.swift (1 hunks)
  • Sources/App/Controllers/Portal/LoginController.swift (1 hunks)
  • Sources/App/Controllers/Portal/LogoutController.swift (1 hunks)
  • Sources/App/Controllers/Portal/PortalController.swift (1 hunks)
  • Sources/App/Controllers/Portal/ResetController.swift (1 hunks)
  • Sources/App/Controllers/Portal/SignupController.swift (1 hunks)
  • Sources/App/Controllers/Portal/VerifyController.swift (1 hunks)
  • Sources/App/Core/Cognito.swift (1 hunks)
  • Sources/App/Core/Dependencies/CognitoClient.swift (1 hunks)
  • Sources/App/Core/SessionAuthentication.swift (1 hunks)
  • Sources/App/Core/SiteURL.swift (8 hunks)
  • Sources/App/Views/Blog/BlogActions+Index+View.swift (2 hunks)
  • Sources/App/Views/Home/HomeIndex+View.swift (2 hunks)
  • Sources/App/Views/NavMenuItems.swift (3 hunks)
  • Sources/App/Views/Plot+Extensions.swift (1 hunks)
  • Sources/App/Views/Portal/ForgotPassword+View.swift (1 hunks)
  • Sources/App/Views/Portal/Login+View.swift (1 hunks)
  • Sources/App/Views/Portal/Portal+View.swift (1 hunks)
  • Sources/App/Views/Portal/Reset+View.swift (1 hunks)
  • Sources/App/Views/Portal/Signup+View.swift (1 hunks)
  • Sources/App/Views/Portal/Successful+Password+Change.swift (1 hunks)
  • Sources/App/Views/Portal/Verify+View.swift (1 hunks)
  • Sources/App/Views/PublicPage.swift (1 hunks)
  • Sources/App/Views/Search/SearchShow+View.swift (2 hunks)
  • Sources/App/configure.swift (2 hunks)
  • Sources/App/routes.swift (1 hunks)
  • Tests/AppTests/AllTests.swift (1 hunks)
  • Tests/AppTests/PortalTests.swift (1 hunks)
  • Tests/AppTests/__Snapshots__/WebpageSnapshotTests/HomeIndex_document_development.1.html (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.swift`: Every file should have the Apache 2.0 License preamble at the top, commented to suit the language.

**/*.swift: Every file should have the Apache 2.0 License preamble at the top, commented to suit the language.

  • Tests/AppTests/AllTests.swift
  • Sources/App/configure.swift
  • Sources/App/Views/Blog/BlogActions+Index+View.swift
  • Sources/App/Views/Home/HomeIndex+View.swift
  • Sources/App/Views/Search/SearchShow+View.swift
  • Sources/App/Controllers/Portal/PortalController.swift
  • Sources/App/Controllers/Portal/DeleteAccountController.swift
  • Sources/App/Controllers/Portal/SignupController.swift
  • Sources/App/Controllers/Portal/ForgotPasswordController.swift
  • Sources/App/Controllers/Portal/VerifyController.swift
  • Sources/App/Controllers/Portal/ResetController.swift
  • Sources/App/Views/NavMenuItems.swift
  • Sources/App/Views/Portal/Signup+View.swift
  • Sources/App/Controllers/Portal/LoginController.swift
  • Sources/App/Views/Portal/ForgotPassword+View.swift
  • Sources/App/Controllers/Portal/LogoutController.swift
  • Sources/App/Core/SessionAuthentication.swift
  • Sources/App/Views/Portal/Verify+View.swift
  • Sources/App/Views/Portal/Reset+View.swift
  • Sources/App/Views/PublicPage.swift
  • Sources/App/Core/Cognito.swift
  • Sources/App/routes.swift
  • Sources/App/Views/Plot+Extensions.swift
  • Sources/App/Core/SiteURL.swift
  • Sources/App/Views/Portal/Successful+Password+Change.swift
  • Sources/App/Core/Dependencies/CognitoClient.swift
  • Sources/App/Views/Portal/Login+View.swift
  • Sources/App/Views/Portal/Portal+View.swift
  • Tests/AppTests/PortalTests.swift
🧬 Code Graph Analysis (10)
Sources/App/Controllers/Portal/PortalController.swift (6)
Sources/App/Controllers/Portal/ResetController.swift (1)
  • show (12-15)
Sources/App/Controllers/Portal/LoginController.swift (1)
  • show (13-16)
Sources/App/Controllers/Portal/ForgotPasswordController.swift (1)
  • show (12-15)
Sources/App/Controllers/Portal/SignupController.swift (1)
  • show (12-15)
Sources/App/Controllers/Portal/VerifyController.swift (1)
  • show (13-16)
Sources/App/Views/PublicPage.swift (1)
  • document (32-41)
Sources/App/Controllers/Portal/DeleteAccountController.swift (3)
Sources/App/Core/Cognito.swift (1)
  • deleteUser (129-148)
Sources/App/Controllers/Portal/LogoutController.swift (1)
  • logout (12-18)
Sources/App/Views/PublicPage.swift (1)
  • document (32-41)
Sources/App/Controllers/Portal/ForgotPasswordController.swift (7)
Sources/App/Controllers/Portal/PortalController.swift (1)
  • show (9-12)
Sources/App/Controllers/Portal/ResetController.swift (2)
  • show (12-15)
  • resetPassword (17-38)
Sources/App/Controllers/Portal/LoginController.swift (1)
  • show (13-16)
Sources/App/Controllers/Portal/SignupController.swift (1)
  • show (12-15)
Sources/App/Controllers/Portal/VerifyController.swift (1)
  • show (13-16)
Sources/App/Views/PublicPage.swift (1)
  • document (32-41)
Sources/App/Core/Cognito.swift (2)
  • forgotPassword (69-87)
  • resetPassword (89-107)
Sources/App/Views/Portal/Signup+View.swift (3)
Sources/App/Views/Portal/Login+View.swift (2)
  • pageTitle (19-21)
  • content (23-35)
Sources/App/Views/Portal/Reset+View.swift (2)
  • pageTitle (20-22)
  • content (24-30)
Sources/App/Views/Plot+Extensions.swift (2)
  • emailField (253-263)
  • passwordField (265-275)
Sources/App/Views/Portal/Verify+View.swift (5)
Sources/App/Views/Portal/ForgotPassword+View.swift (2)
  • pageTitle (19-21)
  • content (23-30)
Sources/App/Views/Portal/Signup+View.swift (2)
  • pageTitle (19-21)
  • content (23-30)
Sources/App/Views/Portal/Login+View.swift (2)
  • pageTitle (19-21)
  • content (23-35)
Sources/App/Views/Portal/Reset+View.swift (2)
  • pageTitle (20-22)
  • content (24-30)
Sources/App/Views/Plot+Extensions.swift (1)
  • confirmationCodeField (277-288)
Sources/App/Views/Portal/Reset+View.swift (1)
Sources/App/Views/Plot+Extensions.swift (3)
  • confirmationCodeField (277-288)
  • emailField (253-263)
  • passwordField (265-275)
Sources/App/Core/Cognito.swift (3)
Sources/App/Core/SessionAuthentication.swift (1)
  • authenticate (18-26)
Sources/App/Controllers/Portal/SignupController.swift (1)
  • signup (17-36)
Sources/App/Controllers/Portal/ResetController.swift (1)
  • resetPassword (17-38)
Sources/App/Views/Portal/Successful+Password+Change.swift (6)
Sources/App/Views/Portal/Verify+View.swift (2)
  • pageTitle (20-22)
  • content (24-35)
Sources/App/Views/Portal/ForgotPassword+View.swift (2)
  • pageTitle (19-21)
  • content (23-30)
Sources/App/Views/Portal/Portal+View.swift (2)
  • pageTitle (19-21)
  • content (23-31)
Sources/App/Views/Portal/Signup+View.swift (2)
  • pageTitle (19-21)
  • content (23-30)
Sources/App/Views/Portal/Login+View.swift (2)
  • pageTitle (19-21)
  • content (23-35)
Sources/App/Views/Portal/Reset+View.swift (2)
  • pageTitle (20-22)
  • content (24-30)
Sources/App/Core/Dependencies/CognitoClient.swift (1)
Sources/App/Core/Cognito.swift (3)
  • authenticateToken (29-47)
  • confirmSignUp (109-127)
  • deleteUser (129-148)
Sources/App/Views/Portal/Portal+View.swift (4)
Sources/App/Views/Portal/Signup+View.swift (2)
  • pageTitle (19-21)
  • content (23-30)
Sources/App/Views/Portal/Login+View.swift (2)
  • pageTitle (19-21)
  • content (23-35)
Sources/App/Controllers/Portal/LogoutController.swift (1)
  • logout (12-18)
Sources/App/Controllers/Portal/DeleteAccountController.swift (1)
  • deleteAccount (13-25)
🪛 SwiftLint (0.57.0)
Sources/App/Core/SessionAuthentication.swift

[Warning] 24-24: TODOs should be resolved (.unauthorized SotoCognitoError...)

(todo)

Sources/App/Core/Cognito.swift

[Warning] 30-30: Returning Void in a function declaration is redundant

(redundant_void_return)

Tests/AppTests/PortalTests.swift

[Warning] 39-39: Prefer non-optional Data(_:) initializer when converting String to Data

(non_optional_string_data_conversion)


[Warning] 68-68: Prefer non-optional Data(_:) initializer when converting String to Data

(non_optional_string_data_conversion)

🪛 GitHub Check: Test
Tests/AppTests/PortalTests.swift

[failure] 285-285:
cannot infer contextual base in reference to member 'POST'


[failure] 284-284:
cannot find 'withApp' in scope


[failure] 251-251:
cannot infer contextual base in reference to member 'POST'


[failure] 250-250:
cannot find 'withApp' in scope


[failure] 199-199:
cannot infer contextual base in reference to member 'POST'


[failure] 198-198:
cannot find 'withApp' in scope


[failure] 147-147:
cannot infer contextual base in reference to member 'unauthorized'


[failure] 144-144:
cannot infer contextual base in reference to member 'POST'


[failure] 143-143:
cannot find 'withApp' in scope

🪛 GitHub Actions: Query Performance Test
Tests/AppTests/PortalTests.swift

[error] 27-410: Multiple errors: 'withApp' is not found in scope; cannot infer contextual base in reference to member 'POST', 'GET', 'unauthorized', 'ok', 'seeOther', 'setCookie', 'internalServerError'; type of expression is ambiguous without a type annotation.

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Release build
🔇 Additional comments (31)
FrontEnd/styles/search.scss (1)

112-115: Approved: Adjusted navbar search max-width
The reduced max-width: 140px complements the updated header layout that includes the portal link. Please verify it displays correctly across breakpoints.

FrontEnd/main.scss (1)

40-42: Verified import of portal styles
Adding @import 'styles/portal' integrates the new portal-specific stylesheet. Its placement after panel_button and before readme maintains logical grouping.

Tests/AppTests/AllTests.swift (1)

99-99: Registered new PortalTests suite
The AllTests extension now includes the PortalTests suite, integrating the portal feature test coverage. The alphabetical ordering and license header are correct.

FrontEnd/styles/header_footer.scss (2)

39-39: Approved: Vertical centering of nav items
Adding align-items: center ensures consistent vertical alignment of navigation links and icons in the header/footer.


95-101: Approved: .portal list item styling
The new flexbox rules for li.portal properly center the portal link. This aligns its presentation with other nav items.

Sources/App/configure.swift (2)

19-21: New Cognito imports added for authentication functionality.

The Cognito imports indicate the addition of AWS-based authentication services to support the new portal feature. These are appropriate for implementing user management capabilities.


82-88: Session management configured with secure settings.

The code properly enables in-memory session storage and configures secure, HTTP-only cookies which help protect against XSS attacks and session hijacking. The isSecure: true flag ensures cookies are only sent over HTTPS connections.

Sources/App/Views/Home/HomeIndex+View.swift (2)

16-16: Added dependencies import.

The dependency injection framework is correctly imported to support environment-based navigation menu customization.


128-135: Environment-based portal menu item display.

The implementation correctly adds the portal menu item for non-production environments only. This is consistent with the approach taken in other view files and allows for feature testing prior to production deployment.

Sources/App/Views/Search/SearchShow+View.swift (2)

16-16: Added dependencies import.

Import of the dependency injection framework is appropriate for accessing environment information.


67-74: Environment-based portal menu item display in search view.

The conditional logic for showing the portal menu item is consistent with the pattern used in other views. The implementation properly uses the environment to determine whether to include the portal menu item.

Sources/App/Views/PublicPage.swift (1)

326-332: Base implementation of environment-based portal menu.

The PublicPage class now conditionally includes the portal menu item for non-production environments. This base implementation ensures consistent behavior across all views that don't override this method.

Note that this class includes .search in its menu items, unlike the overridden implementations in the other view files.

Sources/App/Controllers/Portal/PortalController.swift (1)

8-12: Implementation follows established pattern

The show(req:) method follows the consistent pattern used by other Portal controllers, rendering the view with the current request path and a new model instance.

Sources/App/Controllers/Portal/DeleteAccountController.swift (1)

16-22: Session cleanup follows established pattern

The session cleanup code correctly follows the pattern seen in LogoutController, properly logging out the user, unauthenticating the session, and destroying it before redirection.

Sources/App/Views/Blog/BlogActions+Index+View.swift (1)

111-118: Conditional display of portal navigation item

The implementation correctly adds the portal navigation item conditionally based on the environment, keeping it hidden in production while making it available in other environments for testing and development.

Sources/App/Views/NavMenuItems.swift (2)

24-24: LGTM: New portal menu item case added.

The addition of the portal case to the NavMenuItem enum is appropriate for the new portal feature being introduced.


40-40: Simplified label text.

Changed from "Add a Package" to "Add Package" for a more concise label.

Sources/App/Controllers/Portal/LogoutController.swift (1)

9-20: Logout implementation looks good.

The controller properly handles the logout flow by cleaning up authentication state and destroying the session before redirecting to the home page.

Sources/App/Controllers/Portal/ResetController.swift (1)

12-15: Show method implementation looks good.

The method correctly renders the reset password view with an empty model.

Sources/App/Controllers/Portal/SignupController.swift (1)

12-15: Show method implementation looks good.

The method correctly renders the signup view with an empty error message.

Sources/App/routes.swift (1)

141-143: Session-first grouping is fine, but double-check the redirect path helper

AuthenticatedUser.redirectMiddleware(path: SiteURL.login.relativeURL()) will receive a fully relative URL (e.g. /login) because relativeURL(_:) prepends “/” if it’s missing.
If the middleware expects a path without the leading slash (several Vapor snippets in the codebase do), you’ll silently create //login redirects. Please confirm the middleware’s expectation, or strip the leading slash:

-let redirect = auth.grouped(AuthenticatedUser.redirectMiddleware(path: SiteURL.login.relativeURL()))
+let loginPath = SiteURL.login.path   // "login"
+let redirect = auth.grouped(AuthenticatedUser.redirectMiddleware(path: loginPath))
Sources/App/Views/Plot+Extensions.swift (3)

252-263: Well-structured email field implementation.

The emailField implementation follows the established pattern in this file and provides a standardized way to create email input fields with appropriate attributes for all portal-related forms.


265-275: Good password field implementation with customization options.

The passwordField implementation allows for both a default and custom placeholder text, which adds flexibility while maintaining consistent behavior for password inputs across the portal.


277-288: Proper implementation of confirmation code field.

The confirmationCodeField method creates a standardized input for confirmation codes with the appropriate class for styling. This implementation aligns well with the other input field patterns.

Sources/App/Views/Portal/Verify+View.swift (2)

4-38: Verify view implementation is well-structured.

The view class correctly inherits from PublicPage and properly overrides the necessary methods. The model structure is clean and the content layout follows the pattern established in other portal pages.


39-58: Well-implemented verification form.

The form correctly uses the confirmationCodeField method from the extensions and properly handles form submission with the POST method. Good job on disabling Turbo Drive to ensure proper form submission.

Sources/App/Views/Portal/Portal+View.swift (2)

4-33: Portal view structure is clear and follows established patterns.

The view implementation follows the same structure as other portal pages, with a clean model and content layout that integrates well with the rest of the application.


35-61: Well-implemented logout and delete account buttons.

Both form implementations properly use POST methods and disable Turbo Drive. The button styling is consistent with the portal design.

Sources/App/Core/Cognito.swift (2)

7-28: Authentication method is well-implemented.

The authenticate method correctly creates an AWS client, configures Cognito, performs authentication, and ensures the client is shut down. Good error handling with proper cleanup.


129-149: Consider adding a confirmation step for account deletion.

The deleteUser method directly deletes the user account without confirmation. For a destructive action like this, you might want to consider implementing a confirmation step in the UI workflow to prevent accidental deletions.

Is there a confirmation dialog or secondary check before the delete account form is submitted, or is this handled elsewhere in the codebase?

Sources/App/Core/Dependencies/CognitoClient.swift (1)

20-29: Surface area looks good – one naming / semantics nit

The closure authenticateToken(req:sessionID:accessToken:) suggests both sessionID and accessToken are forwarded to Cognito, yet the current implementation in Cognito.authenticateToken ignores accessToken and passes sessionID as the accessToken argument (see Sources/App/Core/Cognito.swift).
This asymmetry is easy to miss and will bite if you later depend on the real access token.

- authenticateToken: { req, sessionID, accessToken in
-     try await Cognito.authenticateToken(req: req,
-                                         sessionID: sessionID,
-                                         accessToken: accessToken)
+ authenticateToken: { req, sessionID, accessToken in
+     // ⚠️ Confirm that `Cognito.authenticateToken` uses *accessToken*,
+     //     or amend that implementation to forward the correct value.
+     try await Cognito.authenticateToken(req: req,
+                                         sessionID: sessionID,
+                                         accessToken: accessToken)
 }

Please double-check the downstream call and align the parameter semantics.

Comment on lines +57 to +59
<li>
<a href="/portal">Portal</a>
</li>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure <li> uses portal class for styling
The snapshot’s <li> is missing class="portal", so the header styles defined for .portal won’t apply. Update the NavMenuItems view to emit <li class="portal"> for the portal link.

🤖 Prompt for AI Agents
In
Tests/AppTests/__Snapshots__/WebpageSnapshotTests/HomeIndex_document_development.1.html
around lines 57 to 59, the <li> element for the portal link is missing the class
"portal". To fix this, update the NavMenuItems view code that generates this
list item so it includes class="portal" on the <li> element for the portal link,
ensuring the correct styling is applied.

Comment on lines +1 to +14
import Fluent
import Plot
import Vapor
import SotoCognitoAuthenticationKit

extension Portal {

enum PortalController {
@Sendable
static func show(req: Request) async throws -> HTML {
return PortalPage.View(path: req.url.path, model: PortalPage.Model()).document()
}
}
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Missing Apache License preamble

According to the coding guidelines, all Swift files should include the Apache 2.0 License preamble at the top, commented to suit the language. Please add the standard license header.

+// Copyright Dave Verwer, Sven A. Schmidt, and other contributors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
 import Fluent
 import Plot
 import Vapor
 import SotoCognitoAuthenticationKit
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import Fluent
import Plot
import Vapor
import SotoCognitoAuthenticationKit
extension Portal {
enum PortalController {
@Sendable
static func show(req: Request) async throws -> HTML {
return PortalPage.View(path: req.url.path, model: PortalPage.Model()).document()
}
}
}
// Copyright Dave Verwer, Sven A. Schmidt, and other contributors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
import Fluent
import Plot
import Vapor
import SotoCognitoAuthenticationKit
extension Portal {
enum PortalController {
@Sendable
static func show(req: Request) async throws -> HTML {
return PortalPage.View(path: req.url.path, model: PortalPage.Model()).document()
}
}
}
🤖 Prompt for AI Agents
In Sources/App/Controllers/Portal/PortalController.swift at the top of the file
before any imports, add the standard Apache 2.0 License preamble as a Swift
comment block. This should include the copyright notice and license terms
formatted appropriately for Swift source files.

Comment on lines +1 to +9
import Foundation
import Dependencies
import Fluent
import Plot
import Vapor
import SotoCognitoAuthentication
import SotoCognitoIdentityProvider
import SotoCognitoIdentity

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Missing Apache License preamble

According to the coding guidelines, all Swift files should include the Apache 2.0 License preamble at the top, commented to suit the language. Please add the standard license header.

+// Copyright Dave Verwer, Sven A. Schmidt, and other contributors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
 import Foundation
 import Dependencies
 import Fluent
 import Plot
 import Vapor
 import SotoCognitoAuthentication
 import SotoCognitoIdentityProvider
 import SotoCognitoIdentity
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import Foundation
import Dependencies
import Fluent
import Plot
import Vapor
import SotoCognitoAuthentication
import SotoCognitoIdentityProvider
import SotoCognitoIdentity
// Copyright Dave Verwer, Sven A. Schmidt, and other contributors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
import Foundation
import Dependencies
import Fluent
import Plot
import Vapor
import SotoCognitoAuthentication
import SotoCognitoIdentityProvider
import SotoCognitoIdentity
🤖 Prompt for AI Agents
In Sources/App/Controllers/Portal/DeleteAccountController.swift at the top of
the file (lines 1 to 9), add the standard Apache 2.0 License preamble as a Swift
comment block before any import statements. This involves inserting the full
license header text formatted with comment markers appropriate for Swift files
to comply with the project's coding guidelines.

Comment on lines +1 to +8
import Foundation
import Fluent
import Plot
import Vapor
import SotoCognitoAuthentication
import SotoCognitoIdentityProvider
import SotoCognitoIdentity

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Missing Apache 2.0 License preamble.

According to the coding guidelines, every Swift file should have the Apache 2.0 License preamble at the top, commented to suit the language.

Add the license preamble at the top of the file:

+// Copyright Dave Verwer, Sven A. Schmidt, and other contributors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
 import Foundation
 import Fluent
 import Plot
 import Vapor
 import SotoCognitoAuthentication
 import SotoCognitoIdentityProvider
 import SotoCognitoIdentity
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import Foundation
import Fluent
import Plot
import Vapor
import SotoCognitoAuthentication
import SotoCognitoIdentityProvider
import SotoCognitoIdentity
// Copyright Dave Verwer, Sven A. Schmidt, and other contributors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
import Foundation
import Fluent
import Plot
import Vapor
import SotoCognitoAuthentication
import SotoCognitoIdentityProvider
import SotoCognitoIdentity
🤖 Prompt for AI Agents
In Sources/App/Controllers/Portal/LogoutController.swift at the top of the file
(lines 1 to 8), add the Apache 2.0 License preamble as a comment before any
import statements. This involves inserting the standard Apache 2.0 license
header formatted for Swift files, ensuring it is properly commented out and
placed as the very first content in the file.

Comment on lines +1 to +8
import Fluent
import Dependencies
import Plot
import Vapor
import SotoCognitoAuthentication
import SotoCognitoIdentityProvider
import SotoCognitoIdentity

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Missing Apache 2.0 License preamble.

According to the coding guidelines, every Swift file should have the Apache 2.0 License preamble at the top, commented to suit the language.

Add the license preamble at the top of the file:

+// Copyright Dave Verwer, Sven A. Schmidt, and other contributors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
 import Fluent
 import Dependencies
 import Plot
 import Vapor
 import SotoCognitoAuthentication
 import SotoCognitoIdentityProvider
 import SotoCognitoIdentity
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import Fluent
import Dependencies
import Plot
import Vapor
import SotoCognitoAuthentication
import SotoCognitoIdentityProvider
import SotoCognitoIdentity
// Copyright Dave Verwer, Sven A. Schmidt, and other contributors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
import Fluent
import Dependencies
import Plot
import Vapor
import SotoCognitoAuthentication
import SotoCognitoIdentityProvider
import SotoCognitoIdentity
🤖 Prompt for AI Agents
In Sources/App/Controllers/Portal/ResetController.swift at the top of the file,
add the Apache 2.0 License preamble as a comment according to Swift syntax. This
should be placed before any import statements to comply with coding guidelines.

Comment on lines +1 to +3
import Plot
import Foundation

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Missing license header.

The Apache 2.0 License preamble should be included at the top of this file as required by the coding guidelines for .swift files.

Add the following license header:

+// Copyright Dave Verwer, Sven A. Schmidt, and other contributors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
 import Plot
 import Foundation
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import Plot
import Foundation
// Copyright Dave Verwer, Sven A. Schmidt, and other contributors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
import Plot
import Foundation
🤖 Prompt for AI Agents
In Sources/App/Views/Portal/Portal+View.swift at the top of the file (lines 1 to
3), add the Apache 2.0 License preamble as a comment before any import
statements to comply with the project's coding guidelines for .swift files.

Comment on lines +1 to +6
import Vapor
import SotoCognitoAuthentication
import SotoCognitoIdentityProvider
import SotoCognitoIdentity


Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Missing license header.

The Apache 2.0 License preamble should be included at the top of this file as required by the coding guidelines for .swift files.

Add the following license header:

+// Copyright Dave Verwer, Sven A. Schmidt, and other contributors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
 import Vapor
 import SotoCognitoAuthentication
 import SotoCognitoIdentityProvider
 import SotoCognitoIdentity
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import Vapor
import SotoCognitoAuthentication
import SotoCognitoIdentityProvider
import SotoCognitoIdentity
// Copyright Dave Verwer, Sven A. Schmidt, and other contributors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
import Vapor
import SotoCognitoAuthentication
import SotoCognitoIdentityProvider
import SotoCognitoIdentity
🤖 Prompt for AI Agents
In Sources/App/Core/Cognito.swift at the top of the file (lines 1 to 6), add the
Apache 2.0 License preamble as a comment before any import statements to comply
with the project's coding guidelines for .swift files.

Comment on lines +45 to +47
extension CognitoClient: Sendable, TestDependencyKey {
static var testValue: Self { Self() }
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

testValue currently provides unusable stubs

Overriding static var testValue with Self() compiles, but each stored
closure still contains the macro-generated "unimplemented" trap.
Any test that forgets to stub a particular operation will crash at runtime.

Two robust alternatives:

  1. Omit the custom testValue override and rely on the macro’s automatic
    unimplemented stubs (call-site clearly crashes with a helpful message).
  2. Provide sensible no-op defaults so that un-stubbed operations are benign.
-extension CognitoClient: Sendable, TestDependencyKey {
-    static var testValue: Self { Self() }
-}
+extension CognitoClient: Sendable, TestDependencyKey {
+    static var testValue: Self {
+        .init(
+            authenticate: { _,_,_ in .init(authenticated: nil) },
+            authenticateToken: { _,_,_  in },
+            signup: { _,_,_ in },
+            resetPassword: { _,_,_,_ in },
+            forgotPassword: { _,_ in },
+            confirmSignUp: { _,_,_ in },
+            deleteUser: { _ in }
+        )
+    }
+}

This prevents silent crashes and documents expected default behaviour.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
extension CognitoClient: Sendable, TestDependencyKey {
static var testValue: Self { Self() }
}
extension CognitoClient: Sendable, TestDependencyKey {
static var testValue: Self {
.init(
authenticate: { _, _, _ in .init(authenticated: nil) },
authenticateToken: { _, _, _ in },
signup: { _, _, _ in },
resetPassword: { _, _, _, _ in },
forgotPassword: { _, _ in },
confirmSignUp: { _, _, _ in },
deleteUser: { _ in }
)
}
}
🤖 Prompt for AI Agents
In Sources/App/Core/Dependencies/CognitoClient.swift around lines 45 to 47, the
current override of static var testValue returns Self() which results in stored
closures containing unimplemented traps that cause runtime crashes if not
stubbed. To fix this, either remove the custom testValue override to use the
macro-generated unimplemented stubs that clearly crash with helpful messages, or
implement testValue with sensible no-op default closures so un-stubbed
operations do not cause silent crashes and document expected default behavior.

Comment on lines +24 to +37
extension AllTests.PortalTests {

@Test func test_portal_route_protected() async throws {
try await withApp { app in
try await app.test(.GET, "portal", afterResponse: { res async throws in
#expect(res.status == .seeOther)
if let location = res.headers.first(name: .location) {
#expect("/login" == location)
}
})
}
}

@Test func test_login_successful_redirect() async throws {
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

extension AllTests.PortalTests {} may hide tests from XCTest

Declaring tests inside an extension of a type that doesn’t exist in the test
target results in zero tests being discovered.
If AllTests is the Linux test manifest, move the methods into a concrete
final class PortalTests: XCTestCase (or the Testing framework equivalent) or
ensure AllTests.PortalTests is defined elsewhere.

Failure to do so explains why CI cannot resolve symbols – the compilation unit
isn’t what you expect.

🤖 Prompt for AI Agents
In Tests/AppTests/PortalTests.swift around lines 24 to 37, the test methods are
declared inside an extension of AllTests.PortalTests, which may not be
recognized by XCTest if AllTests.PortalTests is not defined in the test target.
To fix this, move the test methods into a concrete final class named PortalTests
that inherits from XCTestCase (or the equivalent testing base class) instead of
using an extension on AllTests.PortalTests. This ensures the tests are properly
discovered and compiled by the test framework.

Comment on lines +15 to +23
@testable import App

import Testing

import Fluent
import Vapor
import Dependencies
import SotoCognitoAuthenticationKit

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Missing test utilities import causes compilation failures

CI shows “cannot find withApp in scope” and ambiguity around .POST.
Both are provided by XCTVapor, not plain Vapor.

-import Testing
-import Fluent
-import Vapor
+import XCTVapor
+import Testing
+import Fluent
+import Vapor

(If withApp lives in a local helper module, import that as well.)
This should resolve the pipeline’s contextual base errors.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@testable import App
import Testing
import Fluent
import Vapor
import Dependencies
import SotoCognitoAuthenticationKit
@testable import App
import XCTVapor
import Testing
import Fluent
import Vapor
import Dependencies
import SotoCognitoAuthenticationKit
🤖 Prompt for AI Agents
In Tests/AppTests/PortalTests.swift around lines 15 to 23, the file is missing
the import of XCTVapor, which provides the `withApp` function and HTTP method
helpers like `.POST`. Add `import XCTVapor` to the import statements to resolve
the "cannot find `withApp` in scope" and `.POST` ambiguity errors. If `withApp`
is defined in a local helper module, import that module as well.

@daveverwer daveverwer closed this May 19, 2025
@daveverwer daveverwer deleted the portal-test branch May 19, 2025 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants