-
Notifications
You must be signed in to change notification settings - Fork 90
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
[frontend/backend]Adding the ability to import/export mappers #1263
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1263 +/- ##
============================================
- Coverage 29.05% 28.51% -0.55%
+ Complexity 1300 1289 -11
============================================
Files 507 510 +3
Lines 13056 13114 +58
Branches 747 748 +1
============================================
- Hits 3794 3740 -54
- Misses 9094 9214 +120
+ Partials 168 160 -8 ☔ View full report in Codecov by Sentry. |
openbas-front/src/admin/components/settings/data_ingestion/ImportUploaderMapper.tsx
Outdated
Show resolved
Hide resolved
@@ -176,4 +177,34 @@ private void updateInjectImporter(List<InjectImporterUpdateInput> injectImporter | |||
}); | |||
} | |||
|
|||
public List<ImportMapperAddInput> exportMappers(List<String> idsToExport) { |
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.
I don't understand why we need to recreate an object here
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.
It's kind of a shortcut : instead of exporting objects that match what is in the database, we export the object that would allow us to recreate the same mapper (which is why the import code is so small). This is why we're converting it into ImportMapperAddInput.
It result in a smaller file but to be honest, I'm kind of unsure if that's the best way to do it as it assumes that the export is made so that it can be imported. What do you think of it ?
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.
We need to talk about it.
We have an existing mechanism to export based on Json annotation. We can challenge this too to check if there are a modern way to do it.
4359308
to
68115ee
Compare
@Dimfacion I used Mixin to homogenise the export mechanism. |
openbas-api/src/main/java/io/openbas/rest/mapper/form/ImportMapperUpdateInput.java
Show resolved
Hide resolved
Code: ok, Tested: ok |
Proposed changes
Related issues
Checklist
Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...