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

Add Hardware Replacement Tracker #206

Open
wants to merge 2 commits into
base: ltm-1.6
Choose a base branch
from

Conversation

sdoiron0330
Copy link

Add a new model to track Hardware Replacements as a part of a device's lifecycle.

As a device reaches the end of its lifecycle, it would be nice to have a list of the recommended replacement hardware types as information readily available for a device.

image

image

image

image

Copy link
Contributor

@progala progala left a comment

Choose a reason for hiding this comment

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

A great addition to the plugin, thanks @sdoiron0330 . I have just a few comments and suggestions.

  • Please add docs explaining how to use this functionality.
  • Update nav group name to "Hardware Lifecycle" from "Hardware Notices"
  • Let's move template for device page to its own inc template. This should simplify the logic and make it easier to maintain.
  • Do you also want to display matching hardware replacements on the Device Type page?

verbose_name="Current Device Type",
blank=True,
null=True,
related_name="current_device_type",
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we could change this to a more intuitive name, e.g device_type_replaced_by ?

I don't feel like the current name conveys the relationship well.

In [5]: dt.current_device_type.all()
Out[5]: <RestrictedQuerySet [<HardwareReplacementLCM: 3550X Replacing 3500X>]>

verbose_name="Current Inventory Item",
blank=True,
null=True,
related_name="current_inventory_item",
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly, perhaps inventory_item_replaced_by .

verbose_name="Replacement Device Type",
blank=True,
null=True,
related_name="replacement_device_type",
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps device_type_replacement_for .

verbose_name="Replacement Inventory Item",
blank=True,
null=True,
related_name="replacement_inventory_item",
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps inventory_item_replacement_for .

@@ -26,6 +26,13 @@

</li>
{% endif %}
{% if replacements %}
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't show up due to the below conditional at the top of the template:

{% if perms.nautobot_device_lifecycle_mgmt.view_hardwarelcm and 
hw_notices %}

In any case, I think we should move it to its own inc template.

@@ -26,6 +26,13 @@

</li>
{% endif %}
{% if replacements %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{% if replacements %}
{% if hw_replacements %}

@@ -26,6 +26,13 @@

</li>
{% endif %}
{% if replacements %}
<li role="presentation">
<a href="#replacements" aria-controls="hardware" role="tab" data-toggle="tab">
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<a href="#replacements" aria-controls="hardware" role="tab" data-toggle="tab">
<a href="#hw_replacements" aria-controls="hardware" role="tab" data-toggle="tab">

{% if replacements %}
<li role="presentation">
<a href="#replacements" aria-controls="hardware" role="tab" data-toggle="tab">
Replacement Options <span class="badge badge-pill badge-primary">{{ replacements.count }}</span>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Replacement Options <span class="badge badge-pill badge-primary">{{ replacements.count }}</span>
Replacement Options <span class="badge badge-pill badge-primary">{{ hw_replacements.count }}</span>

@@ -96,6 +103,26 @@
</div>
</div>

<!-- Defines all hardware notice logic within this tab-panel. -->
<div role="tabpanel" class="tab-pane" id="replacements">
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<div role="tabpanel" class="tab-pane" id="replacements">
<div role="tabpanel" class="tab-pane" id="hw_replacements">

<th>Replacement Device</th>
<th>Use Case</th>
</tr>
{% for replacement in replacements %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{% for replacement in replacements %}
{% for replacement in hw_replacements %}

@bradh11
Copy link
Contributor

bradh11 commented Aug 21, 2023

@sdoiron0330 - Once you've addressed @progala 's suggestions we can target this for a 1.3.3 release and rebase into the next-2.0 branch.

@bradh11 bradh11 changed the base branch from develop to ltm-1.6 October 17, 2023 14:14
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.

3 participants