Skip to content

Conversation

@julianduque
Copy link
Member

Major changes:

  • Update dependencies to latest (Fastify v5)
  • Add real tests
  • Improve the invoke.sh script
  • Use EcmaScript Modules (no CommonJS)
  • Use proper schema validation with Fastify
  • Add eslint and prettier
  • Add Heroku Button

@julianduque julianduque requested a review from Copilot July 15, 2025 03:07
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR modernizes the codebase by moving to ESM, updating to Fastify v5, adding robust schema validation, integrating lint/format tooling, and improving tests and deployment scripts

  • Migrate CommonJS to EcmaScript modules across app, routes, plugins, and tests
  • Introduce JSON schema validation for new endpoints and expand /accounts test coverage
  • Add ESLint/Prettier configuration, update dependencies, and include a Heroku deploy button

Reviewed Changes

Copilot reviewed 17 out of 21 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
package.json Update metadata, bump dependencies, add lint/format scripts
src/app.js Migrate to ESM, configure Swagger & Autoload plugins
src/routes/index.js Convert to ESM, add schema validation for /unitofwork and Data Cloud routes
test/routes/accounts.test.js Convert tests to ESM and expand /accounts test cases
eslint.config.js Add ESLint and Prettier integration
README.md Enhance documentation, add Heroku Deploy button & usage instructions
Comments suppressed due to low confidence (3)

README.md:59

  • The README refers to /api-docs, but by default the Swagger UI is exposed at /documentation. Please update the path to match the actual route.
- **GET /api-docs** - Interactive Swagger UI for API documentation

README.md:60

  • The docs list /health but the code registers /healthcheck. Align the documentation with the code or adjust the route name for consistency.
- **GET /health** - Health check endpoint

src/routes/index.js:77

  • New routes /unitofwork and /handleDataCloudDataChangeEvent have been added, but there are no corresponding tests. Consider adding tests to cover their validation logic and response behavior.
  fastify.post(

package.json Outdated
"name": "applink-getting-started-nodejs",
"version": "1.0.0",
"description": "Heroku AppLink sample app with Fastify",
"description": "Heroku AppLink Sammple App with Fastify",
Copy link

Copilot AI Jul 15, 2025

Choose a reason for hiding this comment

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

There is a typo in the description: "Sammple" should be "Sample".

Suggested change
"description": "Heroku AppLink Sammple App with Fastify",
"description": "Heroku AppLink Sample App with Fastify",

Copilot uses AI. Check for mistakes.
throw new Error(errorMessage);
}

return reply;
Copy link

Copilot AI Jul 15, 2025

Choose a reason for hiding this comment

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

[nitpick] Returning the reply object without sending any payload can be confusing. You might remove this return or explicitly call reply.send() to clarify the intended behavior.

Suggested change
return reply;
reply.send({ success: true });

Copilot uses AI. Check for mistakes.
@julianduque julianduque merged commit b680c07 into main Jul 15, 2025
1 check passed
@julianduque julianduque deleted the jduque/refactor branch July 15, 2025 16:02
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