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: #14044 - Allow regex renaming of unnamed devices #17212

Merged
merged 4 commits into from
Dec 9, 2024

Conversation

DanSheps
Copy link
Member

Fixes: #14044 - Allow regex renaming of unnamed devices

  • Allow regex renaming of unnamed devices (already allowed actually)
  • Catch errors relating to unnamed devices or integrity errors as a result of the rename process

Note: I removed the ^$ requirement as, in my view, as long as there is a valid regex and the regex results in a unique set, we don't need to enforce the ^$ requirement. For example: ^ or .* or . would all be equally as valid.

The main changes here catch both IntegrityError's from the DB and a validation error to ensure regex is used.

* Allow regex renaming of unnamed devices (already allowed actually)
* Catch errors relating to unnamed devices or integrity errors as a result of the rename process
@DanSheps DanSheps added the type: bug A confirmed report of unexpected behavior in the application label Aug 20, 2024
@DanSheps DanSheps self-assigned this Aug 20, 2024
@DanSheps DanSheps requested a review from jeremystretch August 21, 2024 20:18
@jeremystretch jeremystretch removed the type: bug A confirmed report of unexpected behavior in the application label Aug 23, 2024
Copy link
Member

@jeremystretch jeremystretch left a comment

Choose a reason for hiding this comment

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

Might it be simpler to just replace original values of None with an empty string?

netbox/dcim/views.py Outdated Show resolved Hide resolved
Comment on lines 779 to 786
except IntegrityError as e:
messages.error(self.request, ", ".join(e.args))
clear_events.send(sender=self)

except ValidationError as e:
messages.error(self.request, ", ".join(e.messages))
clear_events.send(sender=self)

Copy link
Member

Choose a reason for hiding this comment

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

Are these changes strictly relevant to #14044? If not, we should break them out into a separate bug report.

Copy link
Member Author

Choose a reason for hiding this comment

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

They are. This will throw a validation error if we don't use regex (We can easily dump that though if we want to go to None == '')

The Integrity error will catch any associated integrity errors. I tried to fine a way to separate the integrity error out and only capture an integrity error that relates to the name field but it wasn't easily do-able.

@DanSheps DanSheps requested a review from jeremystretch August 25, 2024 20:56
Copy link
Contributor

This PR has been automatically marked as stale because it has not had recent activity. It will be closed automatically if no further action is taken.

@github-actions github-actions bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Sep 10, 2024
@DanSheps DanSheps removed the pending closure Requires immediate attention to avoid being closed for inactivity label Sep 12, 2024
@jeremystretch
Copy link
Member

AFAICT the only change actually needed to address #14044 is to ensure that a null name is always treated as an empty string. (Currently this is true only when regex is enabled.) Have I missed something?

@DanSheps
Copy link
Member Author

AFAICT the only change actually needed to address #14044 is to ensure that a null name is always treated as an empty string. (Currently this is true only when regex is enabled.) Have I missed something?

I can get that fixed up right away then

Copy link
Contributor

This PR has been automatically marked as stale because it has not had recent activity. It will be closed automatically if no further action is taken.

@github-actions github-actions bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Oct 13, 2024
@DanSheps DanSheps removed the pending closure Requires immediate attention to avoid being closed for inactivity label Oct 15, 2024
Copy link
Contributor

This PR has been automatically marked as stale because it has not had recent activity. It will be closed automatically if no further action is taken.

@github-actions github-actions bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Nov 17, 2024
@DanSheps DanSheps removed the pending closure Requires immediate attention to avoid being closed for inactivity label Nov 18, 2024
@jeremystretch jeremystretch merged commit 674af4d into develop Dec 9, 2024
6 checks passed
@jeremystretch jeremystretch deleted the 14044-bulkrename-nameless-devices branch December 9, 2024 14:27
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.

NoneType AttributeError when Bulk Renaming Nameless Devices
2 participants