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

Fix #12579 create cable and add another error #13007

Merged

Conversation

netopsab
Copy link
Contributor

@netopsab netopsab commented Jun 26, 2023

Fixes: #12579

Add get_extra_addanother_params function to CableEditView which return :

  • previous selected parent object (a_termination_device or a_termination_powerpanel or a_termination_circuit)
  • previous selected termination type (a_termination_type and b_termination_type)

@DanSheps DanSheps self-requested a review June 26, 2023 20:41
@@ -3131,6 +3131,13 @@ def get_object(self, **kwargs):

return obj

def get_extra_addanother_params(self, request):
return {
'termination_a_device': resolve(request.GET.get('return_url')).kwargs.get('pk'),
Copy link
Member

Choose a reason for hiding this comment

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

We can't assume the A side will be a device; it could also be a power feed or circuit termination.

Copy link
Contributor Author

@netopsab netopsab Jun 27, 2023

Choose a reason for hiding this comment

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

Thank you for the review. I have updated this PR as requested.

@DanSheps DanSheps removed their request for review June 27, 2023 12:56
@netopsab netopsab requested a review from jeremystretch June 29, 2023 09:19
Comment on lines 3141 to 3149
pk = resolve(request.GET.get('return_url')).kwargs.get('pk')
a_type = CABLE_TERMINATION_TYPES.get(request.GET.get('a_terminations_type'))

if hasattr(a_type, 'device'):
params.update({'termination_a_device': pk})
elif hasattr(a_type, 'power_panel'):
params.update({'termination_a_powerpanel': pk})
elif hasattr(a_type, 'circuit'):
params.update({'termination_a_circuit': pk})
Copy link
Member

@DanSheps DanSheps Jun 30, 2023

Choose a reason for hiding this comment

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

You could instead do:

Suggested change
pk = resolve(request.GET.get('return_url')).kwargs.get('pk')
a_type = CABLE_TERMINATION_TYPES.get(request.GET.get('a_terminations_type'))
if hasattr(a_type, 'device'):
params.update({'termination_a_device': pk})
elif hasattr(a_type, 'power_panel'):
params.update({'termination_a_powerpanel': pk})
elif hasattr(a_type, 'circuit'):
params.update({'termination_a_circuit': pk})
for key in request.POST:
if 'device' in key or 'power_panel' in key or 'circuit' in key:
params.update({key: request.POST.get(key})

This lets you remove the resolve import and also improves the workflow for people adding multiple cables between two separate devices.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the suggestion. I have updated this PR as requested.

@jeremystretch
Copy link
Member

Thanks @netopsab!

@jeremystretch jeremystretch merged commit 860be78 into netbox-community:develop Jul 6, 2023
@netopsab netopsab deleted the 12579-fix-cable-add-another branch July 7, 2023 17:03
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Connections - Cables - Create and Add Another Error
3 participants