Skip to content

Closes #264 - Add internationalization support #265

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

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

pheus
Copy link
Contributor

@pheus pheus commented May 17, 2025

Pull Request

Related Issue

Closes: #264 – Add internationalization (i18n) support to AccessList models

New Behavior

Introduces internationalization (i18n) to the AccessList models. Field attributes like verbose_name now use Django’s gettext_lazy to support translations.

Also includes:

  • Minor corrections to docstrings and inline comments.
  • Consistency improvements in model field declarations.

Contrast to Current Behavior

Previously, the AccessList models did not support i18n, which limited their usability in non-English environments. All strings were hardcoded in English and not translatable.

Discussion: Benefits and Drawbacks

Benefits:

  • Enables the plugin to support multiple languages by leveraging Django’s built-in i18n system.
  • Aligns the plugin with NetBox and Django best practices.
  • Improves maintainability and prepares for future localization efforts.

Drawbacks:

  • Slightly increases verbosity of model definitions.
  • No functional changes, but some risk in altering model field declarations (requires review).

This change is fully backwards-compatible.

Changes to the Documentation

No documentation changes are necessary at this time. However, i18n support can be mentioned in future plugin documentation updates or translated UI guides.

Proposed Release Note Entry

Added internationalization (i18n) support to AccessList models for improved localization and translation readiness.

Double Check

  • I have explained my PR according to the information in the comments
    or in a linked issue.
  • My PR targets the dev branch.

@pheus pheus force-pushed the feature/264-add-i18n-support-to-accesslist-models branch from 1cc7dcd to a4f9dcf Compare May 17, 2025 08:54
Introduces internationalization (i18n) to AccessList models. Updates field
definitions to use `gettext_lazy` for translations and improves model class
field consistency. Includes minor corrections to docstrings and comments.
@pheus pheus force-pushed the feature/264-add-i18n-support-to-accesslist-models branch from a4f9dcf to 0f5d608 Compare May 17, 2025 09:00
@cruse1977
Copy link
Member

hi @pheus are there other changes in this PR ? - Im seeing some movement in get_model_urls which I wouldn't expect for internationalization support

@pheus
Copy link
Contributor Author

pheus commented May 23, 2025

Hi @cruse1977,

Thank you for taking the time to review the changes.

You're right - the update to get_model_urls is somewhat outside the scope of the linked issue. During the cleanup, I reordered the parameters for clarity and moved the duplicated method to its abstract base class to reduce redundancy across models.

If you'd prefer, I'm happy to split this out into a separate feature request and PR. Alternatively, I can remove it from this PR entirely and revisit it as part of the planned NetBox v4.3 compatibility work, where the method will likely no longer be needed.

Please let me know what works best for you!

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.

[Feature]: Add internationalization (i18n) support to AccessList models
2 participants