Add REST API to manage selected KiCad categories #100
Conversation
… in KiCad. Supports CREATE, DELETE, UPDATE and PARTIAL_UPDATE. For convenience the `default_value_parameter_template` and `footprint_parameter_template` also accept the parameter name.
* Implementing DELETE/destroy function
WalkthroughThe recent updates enhance the KiCad Library Plugin's API by introducing a new router configuration for category management. A dedicated Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant API
participant Serializer
participant Model
User->>API: Request to list categories
API->>Model: Retrieve all categories
Model-->>API: Return category list
API->>Serializer: Serialize category data
Serializer-->>API: Return serialized data
API-->>User: Respond with category list
User->>API: Request to create category
API->>Serializer: Validate and serialize input data
Serializer-->>API: Return validated data
API->>Model: Create new category
Model-->>API: Confirm category creation
API-->>User: Respond with success message
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 Configuration 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: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- inventree_kicad/KiCadLibraryPlugin.py (3 hunks)
- inventree_kicad/serializers.py (1 hunks)
- inventree_kicad/viewsets.py (2 hunks)
Additional context used
Ruff
inventree_kicad/viewsets.py
52-52: Use
isandis notfor type comparisons, orisinstance()for isinstance checks(E721)
55-55: Use
isandis notfor type comparisons, orisinstance()for isinstance checks(E721)
83-83:
.models.PartParameterTemplateimported but unusedRemove unused import:
.models.PartParameterTemplate(F401)
112-112: Use
isandis notfor type comparisons, orisinstance()for isinstance checks(E721)
Additional comments not posted (8)
inventree_kicad/viewsets.py (6)
63-69: LGTM!The
listmethod is correctly implemented and uses the appropriate serializer.
71-77: LGTM!The
retrievemethod is correctly implemented and usesget_object_or_404for validation.
79-80: LGTM!The
partial_updatemethod correctly delegates to theupdatemethod withpartial=True.
82-95: LGTM!The
updatemethod is correctly implemented, handling parameter resolution and using the serializer for validation.Tools
Ruff
83-83:
.models.PartParameterTemplateimported but unusedRemove unused import:
.models.PartParameterTemplate(F401)
97-120: LGTM!The
createmethod is correctly implemented, handling parameter resolution and using the serializer for creation.Tools
Ruff
112-112: Use
isandis notfor type comparisons, orisinstance()for isinstance checks(E721)
122-128: LGTM!The
destroymethod is correctly implemented and usesget_object_or_404for validation.inventree_kicad/KiCadLibraryPlugin.py (1)
Line range hint
166-197: LGTM!The
setup_urlsmethod correctly configures the new API routes for category-related operations.inventree_kicad/serializers.py (1)
491-514: LGTM!The
KicadDetailedCategorySerializerclass is correctly implemented, with a flexible depth adjustment mechanism based on the request method.
inventree_kicad/viewsets.py
Outdated
| def create(self, request): | ||
| from part.models import PartCategory, PartParameterTemplate | ||
|
|
||
| part_category = get_object_or_404(PartCategory, pk=request.data.get('category')) | ||
|
|
||
| validated_data = { | ||
| "category": part_category, | ||
| "default_symbol": request.data.get('default_symbol', ''), | ||
| "default_footprint": request.data.get('default_footprint', ''), | ||
| "default_reference": request.data.get('default_reference', ''), | ||
| } | ||
|
|
||
| # Add PartParameterTemplate keys | ||
| # Allow passing the parameter name instead of the id | ||
| for parameter in ['default_value_parameter_template', 'footprint_parameter_template']: | ||
| key = 'name' if type(request.data.get(parameter)) == str else 'pk' | ||
| validated_data[parameter] = PartParameterTemplate.objects.filter(**{key:request.data.get(parameter)}).first() | ||
|
|
||
| serializer = serializers.KicadDetailedCategorySerializer() | ||
| created_category = serializer.create(validated_data) | ||
|
|
||
| serializer = serializers.KicadDetailedCategorySerializer(created_category) | ||
|
|
||
| return response.Response(serializer.data) |
There was a problem hiding this comment.
Use isinstance() for type checks.
Replace type checks with isinstance() for better readability and performance.
Use this diff to update the type checks:
- if type(name) == int:
+ if isinstance(name, int):
- elif type(name) == str:
+ elif isinstance(name, str):Committable suggestion was skipped due to low confidence.
Tools
Ruff
112-112: Use
isandis notfor type comparisons, orisinstance()for isinstance checks(E721)
inventree_kicad/viewsets.py
Outdated
| def update(self, request, pk=None, **kwargs): | ||
| from .models import SelectedCategory, PartParameterTemplate | ||
|
|
||
| category = get_object_or_404(SelectedCategory, pk=pk) | ||
|
|
||
| for parameter in ['default_value_parameter_template', 'footprint_parameter_template']: | ||
| if parameter in request.data: | ||
| request.data[parameter] = self.get_part_parameter_id_by_name(request.data.pop(parameter)) | ||
|
|
||
| serializer = self.get_serializer(category, data=request.data, partial=kwargs.get('partial', False)) | ||
| serializer.is_valid(raise_exception=True) | ||
| serializer.save() | ||
|
|
||
| return response.Response(serializer.data) |
There was a problem hiding this comment.
Remove unused import.
The import of PartParameterTemplate is unused in this method. Consider removing it.
Remove the unused import:
-from .models import SelectedCategory, PartParameterTemplate
+from .models import SelectedCategoryCommittable 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 update(self, request, pk=None, **kwargs): | |
| from .models import SelectedCategory, PartParameterTemplate | |
| category = get_object_or_404(SelectedCategory, pk=pk) | |
| for parameter in ['default_value_parameter_template', 'footprint_parameter_template']: | |
| if parameter in request.data: | |
| request.data[parameter] = self.get_part_parameter_id_by_name(request.data.pop(parameter)) | |
| serializer = self.get_serializer(category, data=request.data, partial=kwargs.get('partial', False)) | |
| serializer.is_valid(raise_exception=True) | |
| serializer.save() | |
| return response.Response(serializer.data) | |
| def update(self, request, pk=None, **kwargs): | |
| from .models import SelectedCategory | |
| category = get_object_or_404(SelectedCategory, pk=pk) | |
| for parameter in ['default_value_parameter_template', 'footprint_parameter_template']: | |
| if parameter in request.data: | |
| request.data[parameter] = self.get_part_parameter_id_by_name(request.data.pop(parameter)) | |
| serializer = self.get_serializer(category, data=request.data, partial=kwargs.get('partial', False)) | |
| serializer.is_valid(raise_exception=True) | |
| serializer.save() | |
| return response.Response(serializer.data) |
Tools
Ruff
83-83:
.models.PartParameterTemplateimported but unusedRemove unused import:
.models.PartParameterTemplate(F401)
inventree_kicad/viewsets.py
Outdated
| def get_part_parameter_id_by_name(self, name): | ||
| from .models import PartParameterTemplate | ||
|
|
||
| ret = None | ||
| part_parameter = None | ||
|
|
||
| if type(name) == int: | ||
| # an integer was passed to the function -> assume it's the ID already | ||
| ret = name | ||
| elif type(name) == str: | ||
| part_parameter = PartParameterTemplate.objects.filter(name=name).first() | ||
|
|
||
| if part_parameter: | ||
| ret = part_parameter.pk | ||
|
|
||
| return ret |
There was a problem hiding this comment.
Use isinstance() for type checks.
Replace type checks with isinstance() for better readability and performance.
Use this diff to update the type checks:
- if type(name) == int:
+ if isinstance(name, int):
- elif type(name) == str:
+ elif isinstance(name, str):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 get_part_parameter_id_by_name(self, name): | |
| from .models import PartParameterTemplate | |
| ret = None | |
| part_parameter = None | |
| if type(name) == int: | |
| # an integer was passed to the function -> assume it's the ID already | |
| ret = name | |
| elif type(name) == str: | |
| part_parameter = PartParameterTemplate.objects.filter(name=name).first() | |
| if part_parameter: | |
| ret = part_parameter.pk | |
| return ret | |
| def get_part_parameter_id_by_name(self, name): | |
| from .models import PartParameterTemplate | |
| ret = None | |
| part_parameter = None | |
| if isinstance(name, int): | |
| # an integer was passed to the function -> assume it's the ID already | |
| ret = name | |
| elif isinstance(name, str): | |
| part_parameter = PartParameterTemplate.objects.filter(name=name).first() | |
| if part_parameter: | |
| ret = part_parameter.pk | |
| return ret |
Tools
Ruff
52-52: Use
isandis notfor type comparisons, orisinstance()for isinstance checks(E721)
55-55: Use
isandis notfor type comparisons, orisinstance()for isinstance checks(E721)
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- inventree_kicad/viewsets.py (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- inventree_kicad/viewsets.py
|
@flborn thank you very much for your contribution. Would you be able to address the style checks issues and would you be happy to use isinstance() for type checking? |
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/serializers.py (1 hunks)
- inventree_kicad/viewsets.py (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- inventree_kicad/viewsets.py
| class KicadDetailedCategorySerializer(serializers.ModelSerializer): | ||
| """Custom model serializer for a single KiCad category instance""" | ||
|
|
||
| class Meta: | ||
| """Metaclass defining serializer fields""" | ||
| model = SelectedCategory | ||
| fields = [ | ||
| 'pk', | ||
| 'category', | ||
| 'default_symbol', | ||
| 'default_footprint', | ||
| 'default_reference', | ||
| 'default_value_parameter_template', | ||
| 'footprint_parameter_template', | ||
| ] | ||
|
|
||
| def __init__(self, *args, **kwargs): | ||
| super(KicadDetailedCategorySerializer, self).__init__(*args, **kwargs) | ||
| request = self.context.get('request') | ||
| if request and request.method in ["POST", "PUT", "PATCH"]: | ||
| self.Meta.depth = 0 | ||
| else: | ||
| self.Meta.depth = 1 |
There was a problem hiding this comment.
Refactor suggestion for dynamic depth adjustment.
The dynamic adjustment of Meta.depth in the __init__ method could be improved for clarity. Consider using a helper method to encapsulate this logic.
Apply this refactor to improve clarity:
def __init__(self, *args, **kwargs):
super(KicadDetailedCategorySerializer, self).__init__(*args, **kwargs)
self.adjust_depth_based_on_request()
def adjust_depth_based_on_request(self):
"""Adjust the Meta depth based on the request method."""
request = self.context.get('request')
if request and request.method in ["POST", "PUT", "PATCH"]:
self.Meta.depth = 0
else:
self.Meta.depth = 1
|
@flborn would you be able to add your additions to the README.md? This will help others and make them aware of it. |
Added standard REST API Calls to manage the selected KiCad categories.
This makes it possible to create the categories without the need to go through the admin page and the tedious process, in case you want to add a lot.
It supports the
.list(),.retrieve(),.create(),.update(),.partial_update(), and.destroy()functionality.The endpoints are reachable under
/plugin/kicad-library-plugin/api/category/and/plugin/kicad-library-plugin/api/category/<pk>/respectively.The serializer is showing the Inventree category and the parameter templates in detail, instead of just the id.
For convenience, the default value parameter and footprint parameter can be created/updated also via name.
Summary by CodeRabbit
New Features
Bug Fixes
Chores