Demo: add log of user data and public API route.

[WilmsJochen]

💡 PR Summary generated by FirstMate

New API Route and Logging Enhancements

Changes:
New API Route:

• Added /user-data/:id endpoint in exampleRouter.js to fetch user data.

Logging Implementation:

• Integrated logging in ExampleService to log user data retrieval.
• Created NewService class with logging for data fetching operations.

CI Workflow Update:

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

TLDR: Introduced a new user data API route and logging for data retrieval; updated CI workflow to include tests. Focus on exampleRouter.js and exampleService.js for new functionality.

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

[firstmate]

PR Review

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

    ❌ Documentation drift: Update documentation for manual Docker validation after removal from CI.
    ❌ Logging best practices: Change logging level from info to debug for business data.
    ❌ Security best practices: Add grantAccessByPermissionMiddleware to /user-data/:id route.
    ❌ Testing: Implement unit tests for new service functions getById and getDataFromRepo.

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

[firstmate]

Logging business data at info level is not recommended as per the guidelines. Consider using a different log level, such as debug, for logging business data. Change logger.info(user.data); to logger.debug(user.data); in the getUserData method.

(Based on guideline 'No business data on info level')

[firstmate]

The route /user-data/:id does not use the grantAccessByPermissionMiddleware, which is required for security purposes. You should modify the route to include this middleware as follows:

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

This ensures that all routes check permissions properly.

(Based on guideline 'Check permissions with middleware')

[firstmate]

The new service functions getById and getDataFromRepo lack corresponding unit tests, which are essential for ensuring functionality and reliability. You should implement unit tests in the test folder to cover these functions, similar to the existing unit tests in test/test-user.js. Here's a basic structure for your tests:

import NewService from '../src/services/newService.js';

describe('NewService', () => {
    it('should get data by ID', async () => {
        // Your test logic here
    });

    it('should get data from repo', async () => {
        // Your test logic here
    });
});

(Based on guideline 'Unit tests for service functions')

[firstmate]

PR Review

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

    ❌ Logging issues: User data logged at info level; change to debug level.
    ❌ Security issues: Missing permission middleware on /user-data/:id route.
    ❌ Testing gaps: No unit tests for NewService; implement tests in test folder.

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

[firstmate]

Logging user data at info level is against the guideline. Consider using debug level for logging business data. Replace logger.info(user.data); with logger.debug(user.data);.

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

[firstmate]

The route /user-data/:id does not utilize the grantAccessByPermissionMiddleware. You should add this middleware to ensure proper permission checks, like this:

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

(Based on guideline 'Routes should check permissions')

[firstmate]

The newService.js file lacks corresponding unit tests in the test folder. Implement unit tests to cover the functionality of the NewService class, similar to how it's done in test/test-user.py. Here's a basic structure for your test file:

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

describe('NewService', () => {
    test('should get data by ID', async () => {
        // Your test implementation here
    });
});

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