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.

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

Add support for importing inventory and products #179

Merged
merged 11 commits into from
Apr 30, 2024
Merged

Conversation

Wambere
Copy link
Contributor

@Wambere Wambere commented Mar 27, 2024

IMPORTANT: Where possible all PRs must be linked to a Github issue

Part of #157

Built this branch off of #178 so it might look a bit more loaded

Engineer Checklist

  • I have run ./gradlew spotlessApply to check my code follows the project's style guide
  • I have built and run the efsity jar to verify my change fixes the issue and/or does not break the application

@Wambere Wambere requested review from pld and ukanga March 27, 2024 16:03
@Wambere Wambere mentioned this pull request Apr 22, 2024
2 tasks
@Wambere
Copy link
Contributor Author

Wambere commented Apr 22, 2024

@dubdabasoduba please review the resources created for products and inventories here

@pld @ukanga I'm going to use the other PR ( #178 ) for a proper refactor, just trying to unblock this here

@Wambere Wambere changed the title Add support for importing inventory Add support for importing inventory and products Apr 22, 2024
Copy link
Member

@pld pld left a comment

Choose a reason for hiding this comment

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

this looks pretty good, we could probably make it a little tighter but we'd have to get into python internals in a way that would make it less readable, open to ideas, but I'd be comfortable merging this in

importer/main.py Outdated
Comment on lines 735 to 736
for x in reversed(del_indexes):
del payload_obj["resource"]["characteristic"][x]
Copy link
Member

Choose a reason for hiding this comment

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

think you can move this after the conditional, like to line 740

importer/main.py Outdated
Comment on lines 630 to 631
for x in reversed(del_indexes):
del payload_obj["resource"]["characteristic"][x]
Copy link
Member

Choose a reason for hiding this comment

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

can remove entirely if you move this or other one below the conditional

@pld pld merged commit 3a19579 into main Apr 30, 2024
4 checks passed
@pld pld deleted the 157-import-inventory branch April 30, 2024 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants