Skip to content

Conversation

@hoshinotsuyoshi
Copy link
Member

@hoshinotsuyoshi hoshinotsuyoshi commented Mar 24, 2025

Adjust migration URL according to changes introduced in 13ad841

example

wrong post example: #974 (comment)

the URL https://liam-erd-web.vercel.app/app/projects/6/migrations/69 is incorrect now. That must be like https://liam-erd-web.vercel.app/app/migrations/69

Issue

  • resolve:

Why is this change needed?

What would you like reviewers to focus on?

Testing Verification

What was done

🤖 Generated by PR Agent at 6707cf1

  • Fixed incorrect migration URL path in postComment function.
  • Updated URL structure to match recent changes.

Detailed Changes

Relevant files
Bug fix
postComment.ts
Update migration URL in `postComment` function                     

frontend/apps/app/src/functions/postComment.ts

  • Updated the migration URL to remove the project ID segment.
  • Ensured the URL structure aligns with recent changes.
  • Adjusted the migrationUrl variable to reflect the new path.
  • +1/-1     

    Additional Notes


    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @changeset-bot
    Copy link

    changeset-bot bot commented Mar 24, 2025

    ⚠️ No Changeset found

    Latest commit: 12b4175

    Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

    This PR includes no changesets

    When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

    Click here to learn what changesets are, and how to add one.

    Click here if you're a maintainer who wants to add a changeset to this PR

    @vercel
    Copy link

    vercel bot commented Mar 24, 2025

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

    Name Status Preview Comments Updated (UTC)
    liam-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 24, 2025 10:24am
    liam-erd-sample ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 24, 2025 10:24am
    4 Skipped Deployments
    Name Status Preview Comments Updated (UTC)
    liam-docs ⬜️ Ignored (Inspect) Visit Preview Mar 24, 2025 10:24am
    test-liam-app ⬜️ Ignored (Inspect) Mar 24, 2025 10:24am
    test-liam-docs ⬜️ Ignored (Inspect) Mar 24, 2025 10:24am
    test-liam-erd-sample ⬜️ Ignored (Inspect) Mar 24, 2025 10:24am

    @supabase
    Copy link

    supabase bot commented Mar 24, 2025

    Updates to Preview Branch (fix-post-comment-url) ↗︎

    Deployments Status Updated
    Database Mon, 24 Mar 2025 10:19:47 UTC
    Services Mon, 24 Mar 2025 10:19:47 UTC
    APIs Mon, 24 Mar 2025 10:19:47 UTC

    Tasks are run on every commit but only new migration files are pushed.
    Close and reopen this PR if you want to apply changes from existing seed or migration files.

    Tasks Status Updated
    Configurations Mon, 24 Mar 2025 10:19:48 UTC
    Migrations Mon, 24 Mar 2025 10:19:52 UTC
    Seeding Mon, 24 Mar 2025 10:19:52 UTC
    Edge Functions Mon, 24 Mar 2025 10:19:52 UTC

    View logs for this Workflow Run ↗︎.
    Learn more about Supabase for Git ↗︎.

    @hoshinotsuyoshi hoshinotsuyoshi marked this pull request as ready for review March 24, 2025 10:15
    @hoshinotsuyoshi hoshinotsuyoshi requested a review from a team as a code owner March 24, 2025 10:15
    @hoshinotsuyoshi hoshinotsuyoshi requested review from FunamaYukina, MH4GF, NoritakaIkeda and junkisai and removed request for a team March 24, 2025 10:15
    @qodo-free-for-open-source-projects
    Copy link
    Contributor

    qodo-free-for-open-source-projects bot commented Mar 24, 2025

    PR Reviewer Guide 🔍

    (Review updated until commit 12b4175)

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Unused Parameter

    The projectId parameter was removed from the destructuring assignment, but it might still be present in the function's payload type definition (ReviewResponse). Consider updating the type definition if needed.

    const { reviewComment, pullRequestId, repositoryId } = payload

    @qodo-free-for-open-source-projects
    Copy link
    Contributor

    qodo-free-for-open-source-projects bot commented Mar 24, 2025

    PR Code Suggestions ✨

    Latest suggestions up to 12b4175
    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Learned
    best practice
    Add input validation for payload properties to prevent runtime errors and ensure data integrity

    The code extracts values from the payload without validating that they exist and
    are in the correct format. Missing validation could lead to runtime errors if
    any of these values are undefined, null, or in an incorrect format. Add
    validation checks before using these values.

    frontend/apps/app/src/functions/postComment.ts [12]

    +// Validate payload properties
     const { reviewComment, pullRequestId, repositoryId } = payload
    +if (!reviewComment || !pullRequestId || !repositoryId) {
    +  throw new Error('Missing required fields in payload')
    +}
    • Apply this suggestion
    Suggestion importance[1-10]: 6
    Low
    • More

    Previous suggestions

    Suggestions up to commit 6707cf1
    CategorySuggestion                                                                                                                                    Impact
    General
    Handle missing environment variable

    Consider validating that process.env.NEXT_PUBLIC_BASE_URL exists before using it
    to construct the URL, as its absence could lead to malformed URLs.

    frontend/apps/app/src/functions/postComment.ts [47]

    -const migrationUrl = `${process.env.NEXT_PUBLIC_BASE_URL}/app/migrations/${migration.id}`
    +const baseUrl = process.env.NEXT_PUBLIC_BASE_URL || '';
    +const migrationUrl = `${baseUrl}/app/migrations/${migration.id}`
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion adds a fallback for when NEXT_PUBLIC_BASE_URL environment variable is undefined, which prevents potential runtime errors and malformed URLs. This is a good defensive programming practice that improves code robustness.

    Medium

    Adjust migration URL according to changes introduced in 13ad841
    Copy link
    Contributor

    @MH4GF MH4GF left a comment

    Choose a reason for hiding this comment

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

    😄

    @MH4GF MH4GF added this pull request to the merge queue Mar 24, 2025
    Merged via the queue into main with commit 9fefc51 Mar 24, 2025
    20 checks passed
    @MH4GF MH4GF deleted the fix-post-comment-url branch March 24, 2025 11:30
    @hoshinotsuyoshi
    Copy link
    Member Author

    thx!

    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.

    3 participants