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

Fix #7424: make all of Sirepo moderated #7426

Merged
merged 10 commits into from
Jan 28, 2025
Merged

Fix #7424: make all of Sirepo moderated #7426

merged 10 commits into from
Jan 28, 2025

Conversation

e-carlin
Copy link
Member

A trial is now required to access all of Sirepo. Every new user is moderated.

All existing users will be give 30 days access. After that a paid plan will be required.

30 days after this is deployed (after all non-paid users are expired) we can remove moderating for Jupyterhub. Just the moderation for Sirepo will be enough.

A trial is now required to access all of Sirepo. Every new user is moderated.

All existing users will be give 30 days access. After that a paid
plan will be required.

30 days after this is deployed (after all non-paid users are expired)
we can remove moderating for Jupyterhub. Just the moderation
for Sirepo will be enough.
@e-carlin e-carlin requested a review from robnagler January 19, 2025 19:46
Copy link
Member

@robnagler robnagler left a comment

Choose a reason for hiding this comment

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

The code looks good. I have a few naming comments. I haven't tested yet. I'll test when you are done with these changes

sirepo/api_perm.py Outdated Show resolved Hide resolved
sirepo/auth_role.py Outdated Show resolved Hide resolved
sirepo/auth_role.py Outdated Show resolved Hide resolved
sirepo/auth_role_moderation.py Outdated Show resolved Hide resolved
sirepo/auth_role_moderation.py Outdated Show resolved Hide resolved
<div class="row">
<div class="col-sm-6 col-sm-offset-2">
<h1>Plan Required</h1>
<p>An <span data-plans-link="" link-text="active plan"></span> is required to use Sirepo.</p>
Copy link
Member

Choose a reason for hiding this comment

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

Why is it data-plans-link="" and not directive <data-plan-link>active plan</data-plan-link>? I.e. why is the span necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you create directives that way? I don't see any examples

$ grep -ri --exclude-dir='ext' '<data-' ~/src/radiasoft/sirepo/sirepo/package_data/static/
/home/vagrant/src/radiasoft/sirepo/sirepo/package_data/static/js/sirepo-beamline.js:              <data-ng-include src="::iconUrl" data-onload="iconLoaded()"/>

sirepo/package_data/static/js/sirepo-components.js Outdated Show resolved Hide resolved
@@ -557,6 +564,11 @@
}
},
"customErrors": {
"402": {
Copy link
Member

Choose a reason for hiding this comment

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

Good use of 402. That's one code I didn't have memorized.

However, I think this is for all plan, though? I think this exception is "plan-expired" and in the case of a paid plan, it will have different text.

Copy link
Member Author

Choose a reason for hiding this comment

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

My thinking is regardless of what plan (trial or premium) a user will see the same message. Do you think they should see different messages?
Screenshot 2025-01-22 at 2 44 19 PM

sirepo/util.py Outdated Show resolved Hide resolved
@robnagler
Copy link
Member

Moderation Request
Please describe your reason for requesting access:

On the moderation page it should explain a bit about the reason for the moderation. If it is for require_plan, then it should be explain a bit about the reason why sirepo is moderated.

On the registration page, we'll definitely want something about using institutional email, not gmail or yahoo.

Copy link
Member

@robnagler robnagler left a comment

Choose a reason for hiding this comment

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

I did some simple tests which worked. I will test more on alpha

sirepo/auth/__init__.py Show resolved Hide resolved
sirepo/auth_role_moderation.py Outdated Show resolved Hide resolved
sirepo/auth_role_moderation.py Show resolved Hide resolved
sirepo/const.py Show resolved Hide resolved
sirepo/db_upgrade.py Outdated Show resolved Hide resolved
sirepo/auth_role_moderation.py Outdated Show resolved Hide resolved
sirepo/pkcli/roles.py Show resolved Hide resolved
sirepo/pkcli/roles.py Show resolved Hide resolved
@robnagler robnagler self-requested a review January 28, 2025 02:40
@e-carlin
Copy link
Member Author

@robnagler ready for another review

@robnagler robnagler enabled auto-merge (squash) January 28, 2025 17:27
@robnagler robnagler merged commit 1e285ba into master Jan 28, 2025
3 checks passed
@robnagler robnagler deleted the 7424-trial branch January 28, 2025 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants