Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

using the default tenant for the events insertion endpoint #3748

Merged
merged 1 commit into from
Oct 23, 2024
Merged

using the default tenant for the events insertion endpoint #3748

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/device-registry/controllers/create-event.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,6 @@
addValues: async (req, res, next) => {
try {
logText("adding values...");
const { tenant } = req.query;
const measurements = req.body;
const errors = extractErrorsFromRequest(req);
if (errors) {
Expand All @@ -502,7 +501,11 @@
? defaultTenant
: req.query.tenant;

let result = await createEventUtil.insert(tenant, measurements, next);
let result = await createEventUtil.insert(

Check warning on line 504 in src/device-registry/controllers/create-event.js

View check run for this annotation

Codecov / codecov/patch

src/device-registry/controllers/create-event.js#L504 

Added line #L504 was not covered by tests
defaultTenant,
measurements,
next
);
Comment on lines +504 to +508
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

💡 Codebase verification

Test coverage needed for tenant functionality

The test suite for the addValues method lacks specific test cases for the tenant handling functionality. While the basic test structure exists, it needs to be enhanced to cover:

  1. Default tenant behavior when no tenant is provided
  2. Custom tenant usage when explicitly provided
  3. Proper tenant propagation to createEventUtil.insert
  • Add test case: should use default tenant when no tenant is provided
  • Add test case: should use provided tenant when available
  • Add test case: should properly pass tenant to createEventUtil.insert
🔗 Analysis chain

Implementation looks good, but needs test coverage.

The changes correctly implement the default tenant functionality by using the defaultTenant variable in the createEventUtil.insert call. However, this critical change lacks test coverage.

Let's verify the test coverage:

Would you like me to help create test cases to cover:

  1. Default tenant being used when no tenant is provided
  2. Custom tenant being used when provided
  3. Error handling scenarios

Let me gather more context about the test coverage and implementation details.

This will help us understand:

  1. The exact signature of the insert function
  2. How defaultTenant is initialized
  3. Current test coverage for tenant-related scenarios

Let me gather more specific information about the test coverage for the tenant-related functionality.

This will help us understand:

  1. If there are specific test cases for tenant handling
  2. How the createEventUtil.insert mock is set up
  3. The test environment setup for addValues tests
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for test files related to create-event controller
fd -e test.js -e spec.js create-event

# Search for existing test cases covering the addValues method
rg -A 5 "describe.*addValues|test.*addValues|it.*addValues"

Length of output: 3322

Script:

#!/bin/bash
# Check the implementation of createEventUtil.insert to understand its parameters
ast-grep --pattern 'insert = async function($$$) {'

# Look for the defaultTenant initialization
rg -B 2 -A 2 'defaultTenant.*=.*constants\.DEFAULT_TENANT'

# Check test setup for tenant-related configurations
rg -B 5 'tenant.*=.*' src/device-registry/controllers/test/ut_create-event.js

Length of output: 200469

Script:

#!/bin/bash
# Look for test cases specifically related to tenant handling in create-event tests
rg -B 2 -A 2 "tenant.*=.*|defaultTenant" src/device-registry/controllers/test/ut_create-event.js

# Check the test setup for createEventUtil.insert mocks
rg -B 2 -A 2 "createEventUtil.insert.*=.*|insert.*=.*mock" src/device-registry/controllers/test/ut_create-event.js

# Look for beforeEach setup in the addValues test suite
rg -B 5 -A 10 "describe.*addValues" src/device-registry/controllers/test/ut_create-event.js

Length of output: 661

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 504-504: src/device-registry/controllers/create-event.js#L504
Added line #L504 was not covered by tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Create a Github issue for this suggestion.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Baalmart, I've created a GitHub issue to track this suggestion: Issue #3749.

🔗 Analysis chain

@Baalmart, I've opened a GitHub issue to track this suggestion for adding test coverage to the addValues method.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Create a GitHub issue for the test coverage suggestion
gh issue create \
  --title "Add test coverage for tenant functionality in addValues method" \
  --body "The test suite for the \`addValues\` method lacks specific test cases for tenant handling functionality.

See PR #3748 and the discussion [here](https://github.com/airqo-platform/AirQo-api/pull/3748#discussion_r1813683540).

Requested by @Baalmart." \
  --assignee Baalmart

Length of output: 422


if (isEmpty(result) || res.headersSent) {
return;
Expand Down
Loading