Skip to content

Add basic client lib tests#601

Merged
fomalhautb merged 1 commit intodevfrom
client-tests
Apr 3, 2025
Merged

Add basic client lib tests#601
fomalhautb merged 1 commit intodevfrom
client-tests

Conversation

@fomalhautb
Copy link
Copy Markdown
Contributor

@fomalhautb fomalhautb commented Apr 3, 2025


Important

Add basic client library tests and refactor URL construction for improved error handling.

  • Tests:
    • Added app.test.ts to test project scaffolding, user sign-up, and server user creation.
    • Removed general.test.ts as its functionality is now covered in app.test.ts.
  • Helpers:
    • Added createApp() in js-helpers.ts to initialize StackAdminApp, StackClientApp, and StackServerApp.
  • URL Construction:
    • Updated constructRedirectUrl() in url.ts to include callbackUrlName parameter for better error messages.
    • Modified signInWithOAuth(), addNewOAuthProviderOrScope(), and callOAuthCallback() in auth.ts to use updated constructRedirectUrl().
  • Client App Implementation:
    • Updated _StackClientAppImplIncomplete in client-app-impl.ts to use constructRedirectUrl() with callbackUrlName in various methods.
  • Server App Implementation:
    • Updated _StackServerAppImplIncomplete in server-app-impl.ts to use constructRedirectUrl() with callbackUrlName in various methods.

This description was created by Ellipsis for 2b24a64. It will automatically update as commits are pushed.

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 3, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
stack-backend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 3, 2025 4:22pm
stack-dashboard ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 3, 2025 4:22pm
stack-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 3, 2025 4:22pm

@recurseml
Copy link
Copy Markdown

recurseml Bot commented Apr 3, 2025

⚠️ Only 5 files will be analyzed due to processing limits.

@fomalhautb fomalhautb requested a review from N2D4 April 3, 2025 16:18
@recurseml
Copy link
Copy Markdown

recurseml Bot commented Apr 3, 2025

✨ No issues found! Your code is sparkling clean! ✨

🗒️ View all ignored comments in this repo
  • The constraint 'TokenStoreType extends string' is too restrictive. It should likely be 'TokenStoreType extends string | object' to match the condition check in line 113 where TokenStoreType is checked against {}
  • Return type mismatch - the interface declares useUsers() returning ServerUser[] but the Team interface that this extends declares useUsers() returning TeamUser[]
  • There is a syntax error in the super constructor call due to the ellipsis operator used incorrectly. Objects aren't being merged correctly. This syntax usage can lead to runtime errors when trying to pass the merged object to 'super()'. Verify that the intended alterations to the object occur before or outside of the super() call if needed.
  • Throwing an error when no active span is found is too aggressive. The log function should gracefully fallback to console.log or another logging mechanism when there's no active span, since not all execution contexts will have an active span. This makes the code less resilient and could break functionality in non-traced environments.

📚 Relevant Docs

  • Function sets backendContext with a new configuration but doesn't pass 'defaultProjectKeys'. Since defaultProjectKeys is required in the type definition and cannot be updated (throws error if tried to set), this will cause a type error.
  • The schema is using array syntax for pick() which is incorrect for Yup schemas. The pick() method in Yup expects individual arguments, not an array. Should be changed to: emailConfigSchema.pick('type', 'host', 'port', 'username', 'sender_name', 'sender_email')

📚 Relevant Docs

  • Creating a refresh token with current timestamp as expiration means it expires immediately. Should set a future date for token expiration.
  • The 'tools' object is initialized as an empty object, even though 'tools' is presumably expected to contain tool definitions. This could cause the server capabilities to lack necessary tool configurations, thus potentially impacting functionalities that depend on certain tool setups.

📚 Relevant Docs

  • 'STACK_SECRET_SERVER_KEY' is potentially being included in every request header without checking its existence again here. Although it's checked during initialization, this could lead to security issues as it's exposed in all communications where the header is logged or captured.

📚 Relevant Docs

  • When adding 'use client' directive at the beginning, it doesn't check if file.text already contains the 'use client' directive. This could lead to duplicate 'use client' directives if the file already has one.

📚 Relevant Docs

@patched-codes
Copy link
Copy Markdown

patched-codes Bot commented Apr 3, 2025

Documentation Changes Required

  1. docs/fern/docs/pages-template/concepts/stack-app.mdx

    This file needs to be updated to better reflect the different types of Stack apps and their initialization:

    a. Update the code example at the bottom to include StackClientApp and StackAdminApp examples.

    b. Expand the last paragraph discussing StackAdminApp. It currently states that it's "rarely used", but the code shows it's an integral part of project scaffolding and app creation.

    c. Add explanation of StackAdminApp capabilities:

    • Project configuration management
    • API key creation and management
    • Project scaffolding and setup
    • User management with elevated permissions

    d. Note that StackAdminApp requires a super secret admin key and should only be used in secure environments for administrative tasks.

  2. Internal Implementation Changes (No Documentation Updates Required)

    The following changes do not require documentation updates as they affect internal implementation details not documented in the public API:

    a. Changes in packages/template/src/lib/auth.ts:

    • Modifications to the OAuth redirect URL construction process
    • Affected functions: signInWithOAuth, addNewOAuthProviderOrScope, and callOAuthCallback
    • The constructRedirectUrl function now requires an additional parameter

    b. Changes in server-app-impl.ts:

    • Reordering of imports
    • Modified implementation of sendVerificationEmail and inviteUser methods
    • Removed error checks for callback URLs
    • Removed duplicate imports for ProjectPermissionDefinitionsCrud and ProjectPermissionsCrud

Please ensure these changes are reflected in the relevant documentation files.

@fomalhautb fomalhautb merged commit c2cb2aa into dev Apr 3, 2025
17 checks passed
@fomalhautb fomalhautb deleted the client-tests branch April 3, 2025 18:05
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.

3 participants