-
Notifications
You must be signed in to change notification settings - Fork 9
Bulk import functionality for telemetry devices #1550
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.
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
Conversation
Openshift URLs for the PR Deployment: |
Currently duplicate records are just notified about, and the user has to come in and remove those duplicates. do we want the system to filter them out? I also think I copied over the .test files from telemetry import when I created all the device import files and then I never modified them. So i will either have to make the tests work properly or I need to delete these totally 100% duplicate files. |
api/src/paths/project/{projectId}/survey/{surveyId}/devices/import.ts
Outdated
Show resolved
Hide resolved
Looks good! I'll test it more extensively on Monday but the code looks correct - nice work! The test files should basically be identical to the other csv import tests, they'll just call different functions and use different mock data. I can get the tests working if they're currently failing. We should give users as few surprises as possible, so silently removing duplicates might be bad. Showing an error is probably better |
api/src/paths/project/{projectId}/survey/{surveyId}/devices/import.ts
Outdated
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #1550 +/- ##
==========================================
+ Coverage 33.10% 33.15% +0.04%
==========================================
Files 1031 1032 +1
Lines 64952 65053 +101
Branches 2358 2362 +4
==========================================
+ Hits 21503 21568 +65
- Misses 42911 42945 +34
- Partials 538 540 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
|
Description of Changes