-
Notifications
You must be signed in to change notification settings - Fork 65
Added a read only URL field when the strategy is receive #25
Conversation
@rohithasrk need to add explicit Also don't forget to load the static files by specifying the Please rename |
1 similar comment
Now use django.urls.reverse to get the URL, don't forget to append the key on its querystring |
django_netjsongraph/admin.py
Outdated
js = [static('netjsongraph/receive-url.js')] | ||
|
||
def receive_url(self, obj): | ||
return reverse('receive_topology/', kwargs={'pk': obj.key}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wrong, please use obj.pk and append obj.key as its querystring
5 similar comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget to add a test.
See this test as a reference: https://github.com/netjson/django-netjsongraph/blob/master/django_netjsongraph/tests/test_admin.py#L81-L86
django_netjsongraph/admin.py
Outdated
def receive_url(self, obj): | ||
return reverse('receive_topology/', kwargs={'pk': obj.key}) | ||
|
||
receive_url.short_description = "RECEIVE_URL" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use a human readable string translated with gettext, eg: _('receive url')
django_netjsongraph/admin.py
Outdated
def receive_url(self, obj): | ||
return reverse('receive_topology/', kwargs={'pk': obj.pk}) + obj.key | ||
|
||
receive_url.short_description = "RECEIVE_URL" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use a human readable string translated with gettext, eg: _('receive url')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Thanks
django_netjsongraph/admin.py
Outdated
js = [static('netjsongraph/receive-url.js')] | ||
|
||
def receive_url(self, obj): | ||
return reverse('receive_topology/', kwargs={'pk': obj.pk}) + obj.key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrong again, use:
return '{0}?key={1}'.format(reverse('receive_topology/', kwargs={'pk': obj.pk}), obj.key)
References #9.This PR has a lot more work to do. As of now, I only followed the instructions given in #9 comments. There is a problem with my understanding of read only fields in django admin. Here are my doubts:
id = id_<parameter>
? This is what I was able to make out checking the code.@nemesisdesign Please review and study material suggestions are requested.