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

Feature/location types #5588

Merged
merged 11 commits into from
Oct 11, 2023

Conversation

wolflu05
Copy link
Contributor

@wolflu05 wolflu05 commented Sep 21, 2023

This PR implements the basic functionality of StockLocation types like proposed in #5561 .

I'm not sure how far PUI is, but as far as I see, there is currently nothing where I could implement this, so it can be done when implementing the stock location forms and settings.

To keep backwards compatibility I renamed the model field (but kept the db field named icon) to custom_icon and added a setter / getter approach to the icon field. But there is one drawback with this setter, if the icon field gets loaded as default value by any form which is later saved, the location no longer inherits its icon from the location type, because the icon is now saved to the location. Let me know if I should remove the setter or if you have a better idea.

Todo

  • tests
  • bump api version

fixes #5561

@netlify
Copy link

netlify bot commented Sep 21, 2023

Deploy Preview for inventree ready!

Name Link
🔨 Latest commit 51e91db
🔍 Latest deploy log https://app.netlify.com/sites/inventree/deploys/6523014afe215e0008648b73
😎 Deploy Preview https://deploy-preview-5588--inventree.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 100
Accessibility: 86
Best Practices: 100
SEO: 70
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@wolflu05 wolflu05 added api Relates to the API stock Related to Stock models labels Sep 21, 2023
@wolflu05
Copy link
Contributor Author

This would be ready from my POV. The CI issues are as far as I see related to the exchange host api which currently is not working, see Formicka/exchangerate.host#236

@mlucchelli
Copy link

RecursionError: maximum recursion depth exceeded

@wolflu05
Copy link
Contributor Author

RecursionError: maximum recursion depth exceeded

If you look at other test runs from other branches, you can see that the recursion error is present there too in combination with the two exchange rate tests that are failing.

@SchrodingersGat SchrodingersGat added the enhancement This is an suggested enhancement or new feature label Oct 11, 2023
@SchrodingersGat SchrodingersGat added this to the 0.13.0 milestone Oct 11, 2023
@SchrodingersGat SchrodingersGat self-assigned this Oct 11, 2023
@SchrodingersGat
Copy link
Member

@wolflu05 this looks great, thanks for taking the time to implement this, I think it is a very clean implementation :)

@SchrodingersGat SchrodingersGat merged commit e8e0b57 into inventree:master Oct 11, 2023
21 checks passed
SchrodingersGat added a commit to SchrodingersGat/InvenTree that referenced this pull request Oct 11, 2023
@wolflu05 wolflu05 deleted the feature/location-types branch February 15, 2024 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Relates to the API enhancement This is an suggested enhancement or new feature stock Related to Stock models
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FR] Storage location types
3 participants