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

feat/platform support for hardware details #876

Merged
merged 2 commits into from
Feb 4, 2025

Conversation

WilsonNet
Copy link
Collaborator

@WilsonNet WilsonNet commented Feb 3, 2025

@WilsonNet WilsonNet force-pushed the feat/platform-support-for-hardware-details branch from 8023b0c to d43dbb1 Compare February 4, 2025 13:10
@WilsonNet WilsonNet marked this pull request as ready for review February 4, 2025 13:11
Use the database type for it, it was breaking when fetching hardwares
that had that version
@WilsonNet WilsonNet force-pushed the feat/platform-support-for-hardware-details branch from d43dbb1 to 378f588 Compare February 4, 2025 13:52
Copy link
Collaborator

@MarceloRobert MarceloRobert left a comment

Choose a reason for hiding this comment

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

Seems to be working well

@@ -94,7 +96,7 @@ class HardwareTestHistoryItem(TestHistoryItem):
class HardwareBuildHistoryItem(BuildHistoryItem):
tree_name: Optional[str]
issue_id: Optional[str]
issue_version: Optional[int]
issue_version: Optional[Issue__Version]
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: I don't see a need to change this here, unless you change other fields as well, but it will work nonetheless

Copy link
Contributor

Choose a reason for hiding this comment

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

I have the opinion that in this case specifically (moving to the standardized types) any gradual change is already good. But I agree that if this change was not motivated by any other thing other than simply changing the type, I'd like to see this happen at least in every field of this model

Copy link
Contributor

@murilx murilx left a comment

Choose a reason for hiding this comment

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

django models.Q surprised me because I thought this would be harder to implement. Worked well on my test!

- Added `models.Q` to filter queries by `environment_misc__platform`
- Wrapped `_sanitize_records` and `_format_processing_for_response` in try block
- Updated error response to use `e.json()` in `HardwareDetailsSummaryView`
- Added check for `summaryResponse.data.common.compatibles` length in `HardwareDetails.tsx`
@WilsonNet WilsonNet force-pushed the feat/platform-support-for-hardware-details branch from 378f588 to ac8eadb Compare February 4, 2025 19:50
@WilsonNet WilsonNet merged commit 27704fd into main Feb 4, 2025
5 checks passed
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.

HardwareDetails: get details from platforms
3 participants