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

Fixes: #17126 - Respect the weight unit of the DeviceType when displaying the Device detail #17579

Merged
merged 2 commits into from
Sep 24, 2024

Conversation

bctiemann
Copy link
Contributor

@bctiemann bctiemann commented Sep 23, 2024

Fixes: #17126

Changes device.html to conditionally convert a device's total_weight to pounds if the DeviceType defines the weight in pounds in the weight_unit.

Note that this compares the weight_unit value to the literal string 'lb' or 'kg', rather than to an enum value in the WeightUnitChoiceSet. It would be more DRY to pass the enum to the template context, but less elegant as the data construct does not seem to allow direct access to class attrs in the template, meaning the enum would need to be transformed or deconstructed to pass into the template properly.

Copy link
Collaborator

@arthanson arthanson left a comment

Choose a reason for hiding this comment

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

Might be cleaner - You should be able to modify total_weight in DeviceModel to return the correct converted value, It looks like it is only used in this template. I think you could then add another property to get the weight unit which gets the value from WeightUnitChoices.

It looks like there is also grams and ounces possible though I'm not sure they would ever be used.

@bctiemann
Copy link
Contributor Author

Right, I saw there is converters.py which only has a couple of high-level one-way methods, and I noticed that we had templatetags for some richer and more fine-grained conversions so I opted for that.

I too would prefer to put it in a @property or something similar on the model but that might involve adding or refactoring a whole bunch of the conversion logic to make it more centralizable and not confined to templates.

@bctiemann
Copy link
Contributor Author

@arthanson we have this in rack.html:

            <td>
              {{ object.total_weight|floatformat }} {% trans "Kilograms" %}
              ({{ object.total_weight|kg_to_pounds|floatformat }} {% trans "Pounds" %})
            </td>

I've reworked it to follow this pattern, and also fixed rack.html to show the placeholder if the total rack weight is 0. It's redundant now but at least consistent.

@jeremystretch jeremystretch merged commit cfb5696 into develop Sep 24, 2024
6 checks passed
@jeremystretch jeremystretch deleted the 17126-consistent-weight-in-device-detail branch September 24, 2024 20:53
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.

Weight unit between 'device type' and 'device' is not consistent
3 participants