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.

Sign up for GitHub

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

Jump to bottom

[fix] Added organization in import/export subnet #101 #118

Merged
merged 3 commits into from
Oct 7, 2021
Merged

[fix] Added organization in import/export subnet #101 #118

merged 3 commits into from
Oct 7, 2021

Conversation

codesankalp
Copy link
Member

Added third row as organization whilw importing and exporting subnet.

Fixes #101

@codesankalp codesankalp self-assigned this Sep 28, 2021
@codesankalp
Copy link
Member Author

The tests are remaining.

@codesankalp codesankalp force-pushed the issue-101-organizations-imp-exp-subnet branch from 358522a to eb4349e Compare September 29, 2021 13:41
@coveralls
Copy link

coveralls commented Sep 29, 2021 •
edited
Loading

Coverage Status

Coverage increased (+0.5%) to 99.851% when pulling 8d9edd2 on issue-101-organizations-imp-exp-subnet into f352f18 on master.

@codesankalp codesankalp force-pushed the issue-101-organizations-imp-exp-subnet branch from 2601699 to 15e2f6c Compare September 29, 2021 15:16
@nemesifier nemesifier force-pushed the issue-101-organizations-imp-exp-subnet branch from ae46b0a to 88e3d97 Compare October 6, 2021 21:17
nemesifier
nemesifier requested changes Oct 6, 2021
View reviewed changes
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

I improved the error message and relaxed the test which checked it: 88e3d97 because checking for exact error messages in the tests can backfire, since other people may improve the text.

One issue I see here is that this commit c559a5f ended up here but I believe this should be merged in the other PR.

Let me know what you think.

I would remove c559a5f and merge this, then continue with #111.

codesankalp and others added 3 commits October 7, 2021 19:29
@codesankalp
[fix] Added organization in import/export subnet #101
e90f65d 
Added third row as organization while importing and exporting subnet.

Fixes #101
@codesankalp
[fix] Operator cannot import subnet of other organizations #77
4e069b4 
The organization must be created before importing subnet
which is only possible only if the user has permission
to create organizations.

Fixes #77
@nemesifier @codesankalp
[chores] Improved error message
8d9edd2
@codesankalp codesankalp force-pushed the issue-101-organizations-imp-exp-subnet branch from aef08ca to 8d9edd2 Compare October 7, 2021 14:02
nemesifier
nemesifier approved these changes Oct 7, 2021
View reviewed changes
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

Thanks 👍

@nemesifier nemesifier merged commit 5488ddf into master Oct 7, 2021
@nemesifier nemesifier deleted the issue-101-organizations-imp-exp-subnet branch April 28, 2022 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nemesifier nemesifier nemesifier approved these changes

Assignees

@codesankalp codesankalp

Labels
None yet
Projects
OpenWISP Contributor's Board
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[bug] Import/export subnet feature: what about organizations?
3 participants
@codesankalp @coveralls @nemesifier