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

Remove "closed beta" functionality #849

Closed
brylie opened this issue Apr 20, 2021 · 20 comments · Fixed by #963
Closed

Remove "closed beta" functionality #849

brylie opened this issue Apr 20, 2021 · 20 comments · Fixed by #963
Assignees
@aniket-banyal @shashankks0987
Labels
code quality enhancement help wanted mentoring Issues that need active mentoring.
Milestone
2.0

Comments

@brylie
Copy link
Member

brylie commented Apr 20, 2021 •
edited
Loading

Let's treat CiviWiki as an active project that people might deploy to any number of servers for any purpose. To that end, we probably don't need the "closed beta" feature any longer.

This mainly involves removing the beta_blocker decorator from frontend views.

Related code

Remove the following code.

⚠️ Note Remember to run the makemigrations command after changing the project models.

https://github.com/CiviWiki/OpenCiviWiki/search?p=2&q=beta

Here are a few places to be mindful of:

OpenCiviWiki/project/authentication/urls.py

Line 11 in ff74546

url(r'^beta_register', authentication.beta_register, name='beta_register'),

OpenCiviWiki/project/webapp/static/js/beta_register.js

Line 3 in ff74546

cw.BetaRegisterView = BB.View.extend({

OpenCiviWiki/project/frontend_views/views.py

Line 14 in ff74546

from core.custom_decorators import beta_blocker, login_required, full_account

OpenCiviWiki/project/frontend_views/views.py

Lines 22 to 23 in ff74546

if not a.beta_access:
return HttpResponseRedirect('/beta')

OpenCiviWiki/project/frontend_views/urls.py

Line 6 in ff74546

url(r'^beta$', v.beta_view, name='beta'),

OpenCiviWiki/project/frontend_views/urls.py

Line 17 in ff74546

url(r'^beta_register/(?P<email>[\w.%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,4})/(?P<token>\w{31})$', v.beta_register, name='beta_register'),

OpenCiviWiki/project/authentication/authentication.py

Lines 103 to 104 in ff74546

if not settings.CLOSED_BETA:
account.beta_access = True

OpenCiviWiki/project/authentication/authentication.py

Lines 147 to 148 in ff74546

@require_post_params(params=['username', 'password', 'email', 'beta_token'])
def beta_register(request):

OpenCiviWiki/project/webapp/templates/beta_register.html

Line 1 in ff74546

<!doctype html>

OpenCiviWiki/project/authentication/urls.py

Line 11 in ff74546

url(r'^beta_register', authentication.beta_register, name='beta_register'),

OpenCiviWiki/project/core/custom_decorators.py

Line 24 in ff74546

def beta_blocker(func):

OpenCiviWiki/project/webapp/static/less/beta.less

Line 4 in ff74546

#beta{

OpenCiviWiki/project/civiwiki/settings.py

Line 110 in ff74546

CLOSED_BETA = env("CLOSED_BETA", default=False)

OpenCiviWiki/project/api/models/account.py

Line 124 in ff74546

beta_access = models.BooleanField(default=False)

@brylie brylie added this to the 2.0 milestone Apr 20, 2021
@jithenms
Copy link

jithenms commented Apr 24, 2021 •
edited
Loading

Would love to take on this task as one of my first open-source contributions.

To get a better understanding, the beta_blocker decorator essentially prevents those without beta access from accessing features. Since you're launching CiviWiki as an active project, I understand why you wouldn't need the "closed beta" feature then.

I believe I would just need to remove the decorators from the functions so that everyone has the ability to access all features, correct? Let me know if there any other changes e.g. remove or add functions.

Otherwise, I'll get to work on it!

@brylie
Copy link
Member Author

brylie commented Apr 25, 2021

Thanks for offering to help! 😀

Yes, removing the decorator and setting should be sufficient.

Shall I assign you to the task?

@brylie
Copy link
Member Author

brylie commented Apr 25, 2021

Make sure to remove most/all code related to the closed beta and invitations:

https://github.com/CiviWiki/OpenCiviWiki/search?p=2&q=beta

Here are a few places to be mindful of:

OpenCiviWiki/project/authentication/urls.py

Line 11 in ff74546

url(r'^beta_register', authentication.beta_register, name='beta_register'),

OpenCiviWiki/project/webapp/static/js/beta_register.js

Line 3 in ff74546

cw.BetaRegisterView = BB.View.extend({

OpenCiviWiki/project/frontend_views/views.py

Line 14 in ff74546

from core.custom_decorators import beta_blocker, login_required, full_account

OpenCiviWiki/project/frontend_views/views.py

Lines 22 to 23 in ff74546

if not a.beta_access:
return HttpResponseRedirect('/beta')

OpenCiviWiki/project/frontend_views/urls.py

Line 6 in ff74546

url(r'^beta$', v.beta_view, name='beta'),

OpenCiviWiki/project/frontend_views/urls.py

Line 17 in ff74546

url(r'^beta_register/(?P<email>[\w.%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,4})/(?P<token>\w{31})$', v.beta_register, name='beta_register'),

OpenCiviWiki/project/authentication/authentication.py

Lines 103 to 104 in ff74546

if not settings.CLOSED_BETA:
account.beta_access = True

OpenCiviWiki/project/authentication/authentication.py

Lines 147 to 148 in ff74546

@require_post_params(params=['username', 'password', 'email', 'beta_token'])
def beta_register(request):

OpenCiviWiki/project/webapp/templates/beta_register.html

Line 1 in ff74546

<!doctype html>

OpenCiviWiki/project/authentication/urls.py

Line 11 in ff74546

url(r'^beta_register', authentication.beta_register, name='beta_register'),

OpenCiviWiki/project/core/custom_decorators.py

Line 24 in ff74546

def beta_blocker(func):

OpenCiviWiki/project/webapp/static/less/beta.less

Line 4 in ff74546

#beta{

OpenCiviWiki/project/civiwiki/settings.py

Line 110 in ff74546

CLOSED_BETA = env("CLOSED_BETA", default=False)

OpenCiviWiki/project/api/models/account.py

Line 124 in ff74546

beta_access = models.BooleanField(default=False)

@jithenms
Copy link

jithenms commented Apr 25, 2021 •
edited
Loading

Sure, I'll take it.

I believe I've completed the task and will be submitting a PR. Let me know if there are any errors or additional changes I need to make. Thanks!

This was referenced Apr 25, 2021
Closed
Closed
Closed
@brylie
Copy link
Member Author

brylie commented May 13, 2021

@jithenshriyan I am available for the next hour in a CiviWiki office hours hangout on Codebuddies.org.

@brylie brylie added the mentoring Issues that need active mentoring. label May 13, 2021
@brylie
Copy link
Member Author

brylie commented May 13, 2021 •
edited
Loading

@jithenshriyan please provide an update regarding your status with this issue. Let me know if there is any way I can help.

Edit: my bad. I had forgotten to review your pull request. Overall, great work! I made a couple of small change requests.

@1337god
Copy link

1337god commented May 13, 2021

Is there a working website demo?

@brylie
Copy link
Member Author

brylie commented May 14, 2021

Not currently. We will probably deploy a demo once we are closer to the 2.0 release.

@brylie brylie mentioned this issue Jun 14, 2021
Closed
3 tasks
@brylie
Copy link
Member Author

brylie commented Jun 14, 2021

@shashankks0987 this is another high priority task, which might be worth completing before #926

@shashankks0987
Copy link
Contributor

@brylie Cool I can take this up before #926.
I need some clarification on where the user should be redirected instead of '/beta' ?

@brylie
Copy link
Member Author

brylie commented Jun 14, 2021

Sure thing! Just redirect the user to the front page after logging in. Make sure the page indicates they are logged in, e.g. by displaying a log out button.

@shashankks0987 shashankks0987 mentioned this issue Jun 15, 2021
Closed
14 tasks
@shashankks0987
Copy link
Contributor

@brylie How do you guys access the tables in the database usually? Using \d through psql in the docker container is not showing me any tables

@shashankks0987
Copy link
Contributor

I'm a little confused with something here:
With this diff:

@@ -19,8 +19,6 @@ def base_view(request):
         return TemplateResponse(request, "static_templates/landing.html", {})
 
     a = Account.objects.get(user=request.user)
-    if not a.beta_access:
-        return HttpResponseRedirect("/beta")
     if not a.full_account:
         return HttpResponseRedirect("/setup")
     if "login_user_image" not in request.session.keys():
@@ -61,7 +59,6 @@ def base_view(request):
 
 
 @login_required
-@beta_blocker
 @full_account
 def user_profile(request, username=None):
     if not username:

Whenever I try to login I'm getting a google_api_key missing error:
image

What am I missing here?

@brylie
Copy link
Member Author

brylie commented Jun 15, 2021 •
edited
Loading

It looks like you might be bumping into an issue with Google Maps. Specifically, the /setup route/view has some code related to Google Maps. However, we want to remove Google Maps from our project: #7

Would you consider removing the Google dependency before taking this "closed beta" issue? That would help out other developers too.

@brylie
Copy link
Member Author

brylie commented Jun 18, 2021

Closed by #934

@brylie brylie closed this as completed Jun 18, 2021
@aniket-banyal
Copy link
Contributor

There is still beta_access field in Account

OpenCiviWiki/project/api/models/account.py

Line 123 in aa44eeb

beta_access = models.BooleanField(default=False)

CLOSED_BETA is still being used in these 2 places-

OpenCiviWiki/project/accounts/views.py

Lines 57 to 58 in aa44eeb

if hasattr(settings, 'CLOSED_BETA') and not settings.CLOSED_BETA:
account.beta_access = True

OpenCiviWiki/project/accounts/authentication.py

Lines 151 to 152 in aa44eeb

if hasattr(settings, 'CLOSED_BETA') and not settings.CLOSED_BETA:
account.beta_access = True

Do you want these to be removed?

@brylie
Copy link
Member Author

brylie commented Aug 4, 2021

@Dummy26 yes. Please open a specific PR for those changes. Good catch!

@brylie brylie reopened this Aug 4, 2021
@aniket-banyal
Copy link
Contributor

Sure!

@aniket-banyal
Copy link
Contributor

Should I commit these migration files also?
Screenshot (368)

@brylie
Copy link
Member Author

brylie commented Aug 4, 2021

Yes, migrations get committed to the source control.

@aniket-banyal aniket-banyal mentioned this issue Aug 5, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aniket-banyal aniket-banyal

@shashankks0987 shashankks0987

Labels
code quality enhancement help wanted mentoring Issues that need active mentoring.
Projects
None yet
Milestone
2.0
5 participants
@brylie @1337god @aniket-banyal @shashankks0987 @jithenms