Closed anurag-ks closed 6 years ago
Totals | |
---|---|
Change from base Build 116: | 0.0% |
Covered Lines: | 431 |
Relevant Lines: | 431 |
Totals | |
---|---|
Change from base Build 116: | 0.0% |
Covered Lines: | 431 |
Relevant Lines: | 431 |
@okraits would you like to help me reviewing this?
Name
and Description
were added to the index pages :+1: Cancel
button :+1: New annotations:
/django_ipam
IMO Subnets
should be the first link and Ip Addresses
should be the secondIp addresss
should be Ip addresses
- I think this is django's faultHome > Subnets > Import
and Home > Subnets > x.x.x.x/x > Import
?Thanks for the review @okraits ,
verbose_name_plural
value under Meta
. Is there a difference between Home > Subnets > Import and Home > Subnets > x.x.x.x/x > Import?
- There is no difference actually, the csv import format requires the user to specify the subnet as well, so I think we can remove the import
button from this page - Home > Subnets > x.x.x.x/x
(change view) and keep a single import
button on the change list view.Does an import always create a new subnet or is it also possible to only import ip addresses into an existing subnet? If the former is true, then the Import
button should only be on the subnets index page, if the latter is true, then there needs to be an Import
button also on the subnet change page.
I think the text for the import button on the subnets index page should be Import Subnet/IP Addresses
- plural.
@okraits it should be possible to import addresses into an existing subnet, @anurag-ks do you confirm?
I wouldn't touch the PR further, it's good enough for now. The admin lists available models in alphabetical order, we shouldn't mess with that. In the near feature we will introduce a dashboard and a navigation menu, at the moment our users will have to be satisfied with what django is able to give us with minimum effort.
If the subnet already exists, the system would ignore it and add the IP addresses. If the subnet is not present on the system, it will create it and then add the IP addresses.
I will add this in an improved manner to the README as well. I will be sending a PR later.
@okraits I tried to improve the wording, check if now is better and let us know
@nemesisdesign I have to be picky: why Import Subnet
but Export Subnet Data
?
@okraits right, which one you like most? :-)
@nemesisdesign I prefer Import Subnet
and Export Subnet
because it should be clear that Import/Export is about data anyway :-)
@okraits ok I second that, could you edit here on github and send a PR please? I'm overloaded with other PR and issues to review/merge/implement
@okraits ok I second that, could you edit here on github and send a PR please? I'm overloaded with other PR and issues to review/merge/implement
@nemesisdesign Done with #45 :sunglasses:
Implements and closes #38