Demo - [Guidelines] - Public API route + logging user data + no unit test for new service

[WilmsJochen]

💡 PR Summary generated by FirstMate

New API Route and User Data Logging Implementation

Changes:
New API Route:

• Added /user-data/:id route in exampleRouter.js to retrieve user data.

New Service Class:

• Introduced NewService class in newService.js for handling user data retrieval.
• Implemented getById and getDataFromRepo methods with logging functionality.

Logging Enhancements:

• Integrated logging in exampleService.js to log user data when fetched.

CI Pipeline Update:

• Updated .github/workflows/firstmate.yaml to run tests after package installation.

TLDR: This PR adds a new API route for user data retrieval, introduces a new service class with logging, and updates the CI pipeline to include tests. Focus on the new route and service implementations.

_{^{Generated by FirstMate and automatically updated on every commit.}}

[firstmatebot]

PR Review

⚠️ It seems that you can still improve the quality of your PR:

    ❌ Security issues: Route '/user-data/:id' lacks permission checks; add middleware.
    ❌ Logging practices: Log user data at debug level instead of info level.
    ❌ Testing: NewService class missing unit tests; implement tests for methods.
    ❌ Ci/cd pipeline: Missing unit test execution step in GitHub Actions workflow.

_{^{Generated by Firstmate to make sure you can focus on coding new features.}}

[firstmatebot]

Logging user data at info level can expose sensitive information. Consider using debug level for logging business data. Update the logger statement in exampleService.js to use logger.debug(user.data) instead of logger.info(user.data).

(Based on guideline 'Never log business data on info level')

[WilmsJochen]

Good catch

[firstmatebot]

Thank you for your positive feedback! I'm glad you found the suggestion helpful. If you have any further questions or need assistance with anything else, feel free to ask!

[firstmatebot]

The route '/user-data/:id' lacks permission checks. You should include grantAccessByPermissionMiddleware to ensure security. Update the route definition as follows:

+ router.route("/user-data/:id").get(grantAccessByPermissionMiddleware([API_PERMISSIONS.PUBLIC_ENDPOINT]), exampleController.getById)

(Based on guideline 'Routes should check permissions')

[firstmatebot]

The NewService class lacks unit tests. Implement unit tests in the test folder to ensure the functionality of methods like getById and getDataFromRepo. Based on the existing implemented unit test test/test-user.js, you could implement the new unit test as follows:

import NewService from '../services/newService.js';
import exampleRepo from '../repos/exampleRepo.js';

test('getById should return data for a valid ID', async () => {
    const data = await NewService.getById(1);
    expect(data).toBeDefined();
});

(Based on guideline 'Services should have unit tests')

[firstmatebot]

Your workflow is missing a step to execute unit tests on pull requests. Add a step to run unit tests using the command pnpm test in your .github/workflows/firstmate.yaml file. Here's how you can do this:

- name: Run unit tests
  run: pnpm test

(Based on guideline 'Unit tests executed on pull request')