Attachment refactor #98
Conversation
WalkthroughThe recent changes enhance the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant KiCadLibraryPlugin
participant Part
participant PartAttachment (Modern)
participant PartAttachment (Legacy)
User ->> KiCadLibraryPlugin: import_meta_data(part)
KiCadLibraryPlugin ->>+ Part: Retrieve part details
KiCadLibraryPlugin ->>+ add_attachment: Handle attachments
add_attachment ->> PartAttachment (Modern): Add to modern table (if applicable)
add_attachment ->> PartAttachment (Legacy): Add to legacy table (if applicable)
PartAttachment (Modern) -->> KiCadLibraryPlugin: Attachment added
PartAttachment (Legacy) -->> KiCadLibraryPlugin: Attachment added
Note over KiCadLibraryPlugin,User: Attachment process complete
sequenceDiagram
participant User
participant serializers.py
participant Attachment
User ->> serializers.py: get_datasheet(part)
serializers.py ->>+ Attachment: Filter attachments by case-insensitive "datasheet"
Attachment -->> serializers.py: Return filtered datasheet
serializers.py -->> User: Returned datasheet
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- inventree_kicad/KiCadLibraryPlugin.py (2 hunks)
- inventree_kicad/serializers.py (1 hunks)
Additional context used
Ruff
inventree_kicad/serializers.py
234-236: Use a single
ifstatement instead of nestedifstatements (SIM102)
262-263: Use a single
ifstatement instead of nestedifstatements (SIM102)
Additional comments not posted (2)
inventree_kicad/serializers.py (1)
206-206: Optimized attachment filtering logic.The new implementation using
filter(comment__iexact='datasheet')directly in the database query is a significant improvement over iterating over all attachments. This change enhances both performance and clarity by leveraging the database's capabilities to filter data.inventree_kicad/KiCadLibraryPlugin.py (1)
18-18: Updated import statement.The import statement has been simplified by removing
PartAttachment. This change aligns with the new handling of attachments using a unified approach as described in the PR linked at inventree/InvenTree#7420.
| def add_attachment(self, part_id, link): | ||
| """Add an external link as an attachment for the part. | ||
|
|
||
| Note: We support the 'legacy' and 'modern' attachment tables. | ||
|
|
||
| Ref: https://github.com/inventree/InvenTree/pull/7420 | ||
| """ | ||
|
|
||
| # First, try the 'modern' attachment table | ||
| try: | ||
| from common.models import Attachment | ||
|
|
||
| # Check if there is an existing attachment | ||
| attachment = Attachment.objects.filter( | ||
| model_type='part', | ||
| model_id=part_id, | ||
| comment__iexact='datasheet' | ||
| ) | ||
|
|
||
| if not attachment.exists(): | ||
| Attachment.objects.create( | ||
| model_type='part', | ||
| model_id=part_id, | ||
| link=link, | ||
| comment='Datasheet' | ||
| ) | ||
|
|
||
| return | ||
| except Exception: | ||
| pass | ||
|
|
||
| # Second, try the 'legacy' attachment table | ||
| try: | ||
| from part.models import PartAttachment | ||
|
|
||
| # Check if there is an existing attachment | ||
| attachment = PartAttachment.objects.filter( | ||
| part=part_id, | ||
| comment__iexact='datasheet' | ||
| ) | ||
|
|
||
| if not attachment.exists(): | ||
| PartAttachment.objects.create( | ||
| part=part_id, | ||
| link=link, | ||
| comment='Datasheet' | ||
| ) | ||
|
|
||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Refactor suggestion for the add_attachment method.
The method handles both 'modern' and 'legacy' attachment tables, which is good for compatibility. However, the method contains duplicate logic for checking and creating attachments in two different models. Consider refactoring to reduce duplication and improve maintainability.
- # First, try the 'modern' attachment table
- try:
- from common.models import Attachment
- # Check if there is an existing attachment
- attachment = Attachment.objects.filter(
- model_type='part',
- model_id=part_id,
- comment__iexact='datasheet'
- )
- if not attachment.exists():
- Attachment.objects.create(
- model_type='part',
- model_id=part_id,
- link=link,
- comment='Datasheet'
- )
- return
- except Exception:
- pass
- # Second, try the 'legacy' attachment table
- try:
- from part.models import PartAttachment
- # Check if there is an existing attachment
- attachment = PartAttachment.objects.filter(
- part=part_id,
- comment__iexact='datasheet'
- )
- if not attachment.exists():
- PartAttachment.objects.create(
- part=part_id,
- link=link,
- comment='Datasheet'
- )
- except Exception:
- pass
+ def try_add_attachment(model, **kwargs):
+ """Utility function to attempt adding an attachment if it does not exist."""
+ attachment = model.objects.filter(**kwargs)
+ if not attachment.exists():
+ model.objects.create(**kwargs)
+
+ from common.models import Attachment
+ from part.models import PartAttachment
+
+ # Try adding to the 'modern' attachment table
+ try_add_attachment(Attachment, model_type='part', model_id=part_id, link=link, comment='Datasheet')
+
+ # Fallback to the 'legacy' attachment table
+ try_add_attachment(PartAttachment, part=part_id, link=link, comment='Datasheet')Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def add_attachment(self, part_id, link): | |
| """Add an external link as an attachment for the part. | |
| Note: We support the 'legacy' and 'modern' attachment tables. | |
| Ref: https://github.com/inventree/InvenTree/pull/7420 | |
| """ | |
| # First, try the 'modern' attachment table | |
| try: | |
| from common.models import Attachment | |
| # Check if there is an existing attachment | |
| attachment = Attachment.objects.filter( | |
| model_type='part', | |
| model_id=part_id, | |
| comment__iexact='datasheet' | |
| ) | |
| if not attachment.exists(): | |
| Attachment.objects.create( | |
| model_type='part', | |
| model_id=part_id, | |
| link=link, | |
| comment='Datasheet' | |
| ) | |
| return | |
| except Exception: | |
| pass | |
| # Second, try the 'legacy' attachment table | |
| try: | |
| from part.models import PartAttachment | |
| # Check if there is an existing attachment | |
| attachment = PartAttachment.objects.filter( | |
| part=part_id, | |
| comment__iexact='datasheet' | |
| ) | |
| if not attachment.exists(): | |
| PartAttachment.objects.create( | |
| part=part_id, | |
| link=link, | |
| comment='Datasheet' | |
| ) | |
| except Exception: | |
| pass | |
| def add_attachment(self, part_id, link): | |
| """Add an external link as an attachment for the part. | |
| Note: We support the 'legacy' and 'modern' attachment tables. | |
| Ref: https://github.com/inventree/InvenTree/pull/7420 | |
| """ | |
| def try_add_attachment(model, **kwargs): | |
| """Utility function to attempt adding an attachment if it does not exist.""" | |
| attachment = model.objects.filter(**kwargs) | |
| if not attachment.exists(): | |
| model.objects.create(**kwargs) | |
| from common.models import Attachment | |
| from part.models import PartAttachment | |
| # Try adding to the 'modern' attachment table | |
| try_add_attachment(Attachment, model_type='part', model_id=part_id, link=link, comment='Datasheet') | |
| # Fallback to the 'legacy' attachment table | |
| try_add_attachment(PartAttachment, part=part_id, link=link, comment='Datasheet') |
This PR adds support for the "modern" attachments table in InvenTree, which uses a single database table to store all attachments.
Ref: inventree/InvenTree#7420
Without this PR, the plugin breaks on any installation running off the
0.16.xdev branch.Summary by CodeRabbit