-
Notifications
You must be signed in to change notification settings - Fork 13
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
feat: Import metadata from external instance #2481
feat: Import metadata from external instance #2481
Conversation
Front-end summary Node 18
|
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 can remove Exporter class
I do not understand why we need MetaMetadataImporter service
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 should have a better control over metadata validation. I expect in future to not fail import when metadata fields are not matching. We could also import resources without metadata or import resources in class with metadata.
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 should validate all metadata nested from all parents
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.
Can we have more control over metadata objects that we want to inject into resource?
…-on-import' into feat/aut-3590-add-metadata-check-on-import
…github.com/oat-sa/extension-tao-itemqti into feat/aut-3590-add-metadata-check-on-import
…-on-import' into feat/aut-3590-add-metadata-check-on-import
…github.com/oat-sa/extension-tao-itemqti into feat/aut-3590-add-metadata-check-on-import
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.
- New code is covered by tests (if applicable)
- Tests are running successfully (old and new ones) on my local machine (if applicable)
- New code is respecting code style rules
- New code is respecting best practices
- New code is not subject to concurrency issues (if applicable)
- Feature is working correctly on my local machine (if applicable)
- Acceptance criteria are respected
- Pull request title and description are meaningful
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.
Good job!
- New code is covered by tests (if applicable)
- Tests are running successfully (old and new ones) on my local machine (if applicable)
- New code is respecting code style rules
- New code is respecting best practices
- New code is not subject to concurrency issues (if applicable)
- Feature is working correctly on my local machine (if applicable)
- Acceptance criteria are respected
- Pull request title and description are meaningful
], $result); | ||
} | ||
|
||
public function testExtractFailure(): void |
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.
Failure? For me it does test a specific case when empty properties extracted without errors and results are empty as well. I would consider it as a kind of success cause it returns a valid output :)
…import-metadata feat: add form fields
…github.com/oat-sa/extension-tao-itemqti into feat/aut-3590-add-metadata-check-on-import
Version
There are 0 BREAKING CHANGE, 18 features, 6 fixes |
Ticket: https://oat-sa.atlassian.net/browse/AUT-3590
What's Changed
Dependencies PRs
How to test