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

Added the ability to generate custom_labels with custom ranges. #141

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from

Conversation

djhoward12
Copy link
Collaborator

Added the ability to generate custom labels with custom ranges. Currently supports Alphanumeric, Numbers, NumericAlpha, Letters, Roman Numerals, Greek Letter, Binary, and Hex. Updated table and default views to display custom label information if configured.

Added a custom label tab to floor plan form. Added an option to disable tile movement once placed on a floor plan.

@djhoward12 djhoward12 marked this pull request as draft January 6, 2025 16:33
@msheiny
Copy link

msheiny commented Jan 8, 2025

I'm getting an error trying to onboard nautobot using this app, granted im starting from scratch, reproduction:
1 - Create Location Type
2 - Create Location
3 - Go back to view that Location created in step 2, get the following error trace:

Error trace
Request Method: GET
Request URL: http://0.0.0.0:8080/dcim/locations/98870b98-6058-44b0-8fb4-8b728737ff35/

Django Version: 4.2.17
Python Version: 3.11.9

Template error:
In template /usr/local/lib/python3.11/site-packages/nautobot/core/templates/generic/object_retrieve.html, error at line 47
   column nautobot_floor_plan_floorplan.is_tile_movable does not exist
LINE 1: ...", "nautobot_floor_plan_floorplan"."y_axis_step", "nautobot_...
                                                             ^

   37 :                     </span>
   38 :                 </div>
   39 :             </form>
   40 :         </div>
   41 :         {% endif%}
   42 :         {% endwith %}
   43 :     </div>
   44 : 
   45 :     <div class="pull-right noprint">
   46 :     {% block buttons %}
   47 :          {% plugin_buttons object %} 
   48 :         {% block extra_buttons %}{% endblock extra_buttons %}
   49 :         {% consolidate_detail_view_action_buttons %}
   50 :     {% endblock buttons %}
   51 :     </div>
   52 : 
   53 :     {% block masthead %}
   54 :         <h1>
   55 :             <span class="hover_copy">
   56 :                 <span id="copy_title">{% block title %}{{object.display|default:object}}{% endblock %}</span>
   57 :                 <button type="button" class="btn btn-inline btn-default hover_copy_button" data-clipboard-target="#copy_title">


Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/django/db/models/fields/related_descriptors.py", line 473, in __get__
    rel_obj = self.related.get_cached_value(instance)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/db/models/fields/mixins.py", line 15, in get_cached_value
    return instance._state.fields_cache[cache_name]
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

During handling of the above exception ('floor_plan'), another exception occurred:
  File "/usr/local/lib/python3.11/site-packages/django/db/backends/utils.py", line 89, in _execute
    return self.cursor.execute(sql, params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django_prometheus/db/common.py", line 69, in execute
    return super().execute(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The above exception (column nautobot_floor_plan_floorplan.is_tile_movable does not exist
LINE 1: ...", "nautobot_floor_plan_floorplan"."y_axis_step", "nautobot_...
                                                             ^
) was the direct cause of the following exception:
  File "/usr/local/lib/python3.11/site-packages/django/core/handlers/exception.py", line 55, in inner
    response = get_response(request)
               ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/core/handlers/base.py", line 197, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/views/generic/base.py", line 104, in view
    return self.dispatch(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/nautobot/core/views/mixins.py", line 168, in dispatch
    return super().dispatch(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/views/generic/base.py", line 143, in dispatch
    return handler(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/nautobot/core/views/generic.py", line 126, in get
    return render(request, self.get_template_name(), context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/shortcuts.py", line 24, in render
    content = loader.render_to_string(template_name, context, request, using=using)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/loader.py", line 62, in render_to_string
    return template.render(context, request)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/backends/django.py", line 61, in render
    return self.template.render(context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/base.py", line 175, in render
    return self._render(context)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/test/utils.py", line 112, in instrumented_test_render
    return self.nodelist.render(context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/base.py", line 1005, in render
    return SafeString("".join([node.render_annotated(context) for node in self]))
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/base.py", line 1005, in <listcomp>
    return SafeString("".join([node.render_annotated(context) for node in self]))
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/base.py", line 966, in render_annotated
    return self.render(context)
           ^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/loader_tags.py", line 157, in render
    return compiled_parent._render(context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/test/utils.py", line 112, in instrumented_test_render
    return self.nodelist.render(context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/base.py", line 1005, in render
    return SafeString("".join([node.render_annotated(context) for node in self]))
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/base.py", line 1005, in <listcomp>
    return SafeString("".join([node.render_annotated(context) for node in self]))
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/base.py", line 966, in render_annotated
    return self.render(context)
           ^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/loader_tags.py", line 157, in render
    return compiled_parent._render(context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/test/utils.py", line 112, in instrumented_test_render
    return self.nodelist.render(context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/base.py", line 1005, in render
    return SafeString("".join([node.render_annotated(context) for node in self]))
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/base.py", line 1005, in <listcomp>
    return SafeString("".join([node.render_annotated(context) for node in self]))
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/base.py", line 966, in render_annotated
    return self.render(context)
           ^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/loader_tags.py", line 157, in render
    return compiled_parent._render(context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/test/utils.py", line 112, in instrumented_test_render
    return self.nodelist.render(context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/base.py", line 1005, in render
    return SafeString("".join([node.render_annotated(context) for node in self]))
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/base.py", line 1005, in <listcomp>
    return SafeString("".join([node.render_annotated(context) for node in self]))
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/base.py", line 966, in render_annotated
    return self.render(context)
           ^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/loader_tags.py", line 157, in render
    return compiled_parent._render(context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/test/utils.py", line 112, in instrumented_test_render
    return self.nodelist.render(context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/base.py", line 1005, in render
    return SafeString("".join([node.render_annotated(context) for node in self]))
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/base.py", line 1005, in <listcomp>
    return SafeString("".join([node.render_annotated(context) for node in self]))
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/base.py", line 966, in render_annotated
    return self.render(context)
           ^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/loader_tags.py", line 63, in render
    result = block.nodelist.render(context)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/base.py", line 1005, in render
    return SafeString("".join([node.render_annotated(context) for node in self]))
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/base.py", line 1005, in <listcomp>
    return SafeString("".join([node.render_annotated(context) for node in self]))
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/base.py", line 966, in render_annotated
    return self.render(context)
           ^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/loader_tags.py", line 63, in render
    result = block.nodelist.render(context)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/base.py", line 1005, in render
    return SafeString("".join([node.render_annotated(context) for node in self]))
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/base.py", line 1005, in <listcomp>
    return SafeString("".join([node.render_annotated(context) for node in self]))
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/base.py", line 966, in render_annotated
    return self.render(context)
           ^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/template/library.py", line 237, in render
    output = self.func(*resolved_args, **resolved_kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/nautobot/extras/templatetags/plugins.py", line 67, in plugin_buttons
    return _get_registered_content(obj, "buttons", context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/nautobot/extras/templatetags/plugins.py", line 45, in _get_registered_content
    content = getattr(instance, method)()
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/source/nautobot_floor_plan/template_content.py", line 19, in buttons
    floor_plan = location.floor_plan
                 ^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/db/models/fields/related_descriptors.py", line 481, in __get__
    rel_obj = self.get_queryset(instance=instance).get(**filter_args)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/db/models/query.py", line 633, in get
    num = len(clone)
          ^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/db/models/query.py", line 380, in __len__
    self._fetch_all()
    ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/db/models/query.py", line 1881, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/db/models/query.py", line 91, in __iter__
    results = compiler.execute_sql(
              
  File "/usr/local/lib/python3.11/site-packages/django/db/models/sql/compiler.py", line 1562, in execute_sql
    cursor.execute(sql, params)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/debug_toolbar/panels/sql/tracking.py", line 235, in execute
    return self._record(super().execute, sql, params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/debug_toolbar/panels/sql/tracking.py", line 160, in _record
    return method(sql, params)
           ^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/db/backends/utils.py", line 102, in execute
    return super().execute(sql, params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/db/backends/utils.py", line 67, in execute
    return self._execute_with_wrappers(
           
  File "/usr/local/lib/python3.11/site-packages/django/db/backends/utils.py", line 80, in _execute_with_wrappers
    return executor(sql, params, many, context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/db/backends/utils.py", line 84, in _execute
    with self.db.wrap_database_errors:
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/db/utils.py", line 91, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/db/backends/utils.py", line 89, in _execute
    return self.cursor.execute(sql, params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django_prometheus/db/common.py", line 69, in execute
    return super().execute(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Exception Type: ProgrammingError at /dcim/locations/98870b98-6058-44b0-8fb4-8b728737ff35/
Exception Value: column nautobot_floor_plan_floorplan.is_tile_movable does not exist
LINE 1: ...", "nautobot_floor_plan_floorplan"."y_axis_step", "nautobot_...
                                                             ^


@joewesch
Copy link
Contributor

joewesch commented Jan 8, 2025

@msheiny that sounds like the migrations didn't run.

@msheiny
Copy link

msheiny commented Jan 8, 2025

@msheiny that sounds like the migrations didn't run.

Yeeeeepppp - that's exactly it. Looks good now!

nautobot_floor_plan/forms.py Outdated Show resolved Hide resolved
nautobot_floor_plan/forms.py Show resolved Hide resolved
changes/8.added Outdated Show resolved Hide resolved
nautobot_floor_plan/forms.py Outdated Show resolved Hide resolved
@joewesch
Copy link
Contributor

joewesch commented Jan 8, 2025

The nautobot_floor_plan folder is littered with various "utility" files. Can you please move the following to a utils folder:

  • custom_validators.py
  • label_converters.py
  • label_generator.py
  • svg.py
  • utils.py

@djhoward12
Copy link
Collaborator Author

The nautobot_floor_plan folder is littered with various "utility" files. Can you please move the following to a utils folder:

  • custom_validators.py
  • label_converters.py
  • label_generator.py
  • svg.py
  • utils.py

Moved all but svg.py to utils folder. I feel like svg.py is a core component of floor plan and not just a utility.

@msheiny
Copy link

msheiny commented Jan 9, 2025

I recommend squashing these commits down to make the history cleaner before the final merge.

I was also thinking on the custom range selection, as feature request (i can file it as an issue after this PR) - as a user I really want to see a sample of what those X axis entries will be before I hit update and go back to the floor-plan. The modal is much nicer though in the latest push for the examples 👍

@djhoward12 djhoward12 marked this pull request as ready for review January 10, 2025 15:58
@djhoward12 djhoward12 self-assigned this Jan 15, 2025
Copy link
Collaborator

@glennmatthews glennmatthews left a comment

Choose a reason for hiding this comment

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

I'll freely admit I didn't review this exhaustively, but while quite complex it seems well-documented and thoroughly tested. Hopefully it doesn't end up being too complex to maintain. :-)

nautobot_floor_plan/forms.py Show resolved Hide resolved
nautobot_floor_plan/forms.py Show resolved Hide resolved
nautobot_floor_plan/forms.py Outdated Show resolved Hide resolved
nautobot_floor_plan/migrations/0008_add_axis_step.py Outdated Show resolved Hide resolved
nautobot_floor_plan/utils/utils.py Outdated Show resolved Hide resolved
nautobot_floor_plan/tables.py Outdated Show resolved Hide resolved
nautobot_floor_plan/templatetags/seed_helpers.py Outdated Show resolved Hide resolved
nautobot_floor_plan/tests/test_converters.py Outdated Show resolved Hide resolved
nautobot_floor_plan/tests/test_forms.py Show resolved Hide resolved
nautobot_floor_plan/utils/label_converters.py Show resolved Hide resolved
Copy link

@msheiny msheiny left a comment

Choose a reason for hiding this comment

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

I dropped a few comments with references to code areas that were missing a lot of test coverage in my local tests.

This PR is pretty large though, it might be better to let it by if you confirmed it works locally and then swoop back in another PR to try to get more test coverage. Just worried long-term if anyone wants to go make changes to some of these sections later and they can't be 100% confident whether they broke anything or not.

return grid_number_to_letter(converted_location)


def axis_init_label_conversion(axis_origin, axis_location, step, is_letters):
Copy link

Choose a reason for hiding this comment

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

code coverage tooling feedback:

image

) from e


def axis_clean_label_conversion(axis_origin, axis_label, step, is_letters, custom_ranges=None):
Copy link

Choose a reason for hiding this comment

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

coverage:

image

return str(original_location)


def calculate_letter_range_size(custom_range, converter):
Copy link

Choose a reason for hiding this comment

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

looks like dead code? I couldn't find it's reference in-use though theres a similar method in custom_validators.py -> _calculate_letter_range_size

return labels[:count]

# If custom ranges don't provide enough labels, fall back to default labeling
if len(labels) < count:
Copy link

Choose a reason for hiding this comment

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

Looks like everything under this if statement is not being triggered in tests


return labels

def _generate_default_labels(self, axis, count):
Copy link

Choose a reason for hiding this comment

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

this method isnt being hit in unit tests


return self._generate_default_range(start, step, count, is_letters)

def _generate_default_range(self, start, step, count, is_letters):
Copy link

Choose a reason for hiding this comment

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

this section is not being hit in unit-tests

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.

4 participants