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

Add admin option to configure APN #73

Merged
merged 4 commits into from Oct 28, 2018

Conversation

Projects
None yet
3 participants
@c-w
Copy link
Member

commented Oct 28, 2018

Screenshot of settings page

Resolves #70

@codecov-io

This comment has been minimized.

Copy link

commented Oct 28, 2018

Codecov Report

Merging #73 into master will decrease coverage by 0.44%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #73      +/-   ##
==========================================
- Coverage    72.3%   71.86%   -0.45%     
==========================================
  Files          19       20       +1     
  Lines        1217     1251      +34     
==========================================
+ Hits          880      899      +19     
- Misses        337      352      +15
Impacted Files Coverage Δ
opwen_email_client/webapp/config.py 100% <100%> (ø) ⬆️
opwen_email_client/webapp/forms/email.py 42.85% <100%> (ø)
opwen_email_client/webapp/views.py 48% <31.57%> (-0.12%) ⬇️
opwen_email_client/webapp/forms/settings.py 55% <55%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3046cd2...5c740ed. Read the comment docs.

@c-w c-w merged commit 88407d0 into master Oct 28, 2018

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
pyup.io/safety-ci No dependencies with known security vulnerabilities.
Details

@c-w c-w deleted the admin-edit-wvdial branch Oct 28, 2018

@c-w c-w referenced this pull request Oct 28, 2018

Closed

Add "sync now" button #68

@nzola

This comment has been minimized.

Copy link

commented Oct 28, 2018

Great to see the Settings page. I see the page under the vwdial missing APN option.
I believe only the Admin will have the permissions to the Setting page and when he is not there he can make another user admin within the network.

@c-w

This comment has been minimized.

Copy link
Member Author

commented Oct 28, 2018

@nzola The APN line will be there if the SIM information configured for the device has an APN line in it. This page lets the admin edit the entire wvdial.conf file that was generated for the device. The screenshot simply shows the default wvdial configuration which doesn't contain anything.

You're correct that only admins can view the settings page. Also, since #74 an admin can make another user admin, from the Users page (where admins can also suspend accounts or reset passwords):

Screenshot showing admin promotion option on Users page

Note that for now I've added logic that an admin can't "demote" another admin or change their password to prevent potential issues with one admin locking out themselves or everyone else (which is why next to the admin users marked with a star you can't see the suspend or promote options). Let me know if you'd like me to change this.

@nzola

This comment has been minimized.

Copy link

commented Oct 28, 2018

I like the way to implement the admin logic now. If I understand ; the initial admin of the Lokole can pass his admin power to another user. At this point the initial admin cannot take back his admin power, but only the new admin has the power to give it back to him or to someone else. Is this correct? If yes, then this is the way I would love it to operate.

@c-w

This comment has been minimized.

Copy link
Member Author

commented Oct 28, 2018

Thats correct, except that when the admin makes another user admin, they still keep their admin power; simply now there are two admins. Do you wish to only keep one admin on the device at any time? If so that's easy to implement, just let me know.

@nzola

This comment has been minimized.

Copy link

commented Oct 28, 2018

Let's just keep it as it is now. The implementation results will tell us which direction to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.