-
Notifications
You must be signed in to change notification settings - Fork 24
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
Accept terms of Service at signup #8193
Changes from 17 commits
a44f782
b62721f
94716a7
5607fcc
05eed1c
ae61dc6
5340755
ce2396f
4ea0446
77e2a59
b1bb0b8
a4817e4
f52c528
2602e5b
e5d5f19
db456d0
377d289
8d618c4
6c819e3
1d2ecb2
94be416
d60afaa
ca427bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -116,8 +116,8 @@ webKnossos { | |
Please add the information of the operator to comply with GDPR. | ||
""" | ||
termsOfService { | ||
enabled = false | ||
# The URL will be embedded into an iFrame | ||
enabled = true | ||
knollengewaechs marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# The URL will be linked to or embedded into an iFrame | ||
url = "https://webknossos.org/terms-of-service" | ||
acceptanceDeadline = "2023-01-01T00:00:00Z" | ||
version = 1 | ||
|
@@ -146,7 +146,7 @@ features { | |
discussionBoard = "https://forum.image.sc/tag/webknossos" | ||
discussionBoardRequiresAdmin = false | ||
hideNavbarLogin = false | ||
isWkorgInstance = false | ||
isWkorgInstance = true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. undo me too |
||
recommendWkorgInstance = true | ||
taskReopenAllowedInSeconds = 30 | ||
allowDeleteDatasets = true | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,8 @@ import Store from "oxalis/throttled_store"; | |
import messages from "messages"; | ||
import { setHasOrganizationsAction } from "oxalis/model/actions/ui_actions"; | ||
import { setActiveOrganizationAction } from "oxalis/model/actions/organization_actions"; | ||
import { useFetch } from "libs/react_helpers"; | ||
import { getTermsOfService } from "admin/api/terms_of_service"; | ||
|
||
const FormItem = Form.Item; | ||
const { Password } = Input; | ||
|
@@ -26,6 +28,8 @@ type Props = { | |
function RegistrationFormGeneric(props: Props) { | ||
const [form] = Form.useForm(); | ||
|
||
const terms = useFetch(getTermsOfService, null, []); | ||
|
||
const onFinish = async (formValues: Record<string, any>) => { | ||
await Request.sendJSONReceiveJSON( | ||
props.organizationIdToCreate != null | ||
|
@@ -274,28 +278,57 @@ function RegistrationFormGeneric(props: Props) { | |
</FormItem> | ||
</Col> | ||
</Row> | ||
{props.hidePrivacyStatement ? null : ( | ||
<FormItem | ||
name="privacy_check" | ||
valuePropName="checked" | ||
rules={[ | ||
{ | ||
validator: (_, value) => | ||
value | ||
? Promise.resolve() | ||
: Promise.reject(new Error(messages["auth.privacy_check_required"])), | ||
}, | ||
]} | ||
> | ||
<Checkbox> | ||
I agree to storage and processing of my personal data as described in the{" "} | ||
<a target="_blank" href="/privacy" rel="noopener noreferrer"> | ||
privacy statement | ||
</a> | ||
. | ||
</Checkbox> | ||
</FormItem> | ||
)} | ||
<div className="registration-form-checkboxes"> | ||
{props.hidePrivacyStatement ? null : ( | ||
<FormItem | ||
name="privacy_check" | ||
valuePropName="checked" | ||
rules={[ | ||
{ | ||
validator: (_, value) => | ||
value | ||
? Promise.resolve() | ||
: Promise.reject(new Error(messages["auth.privacy_check_required"])), | ||
}, | ||
]} | ||
> | ||
<Checkbox> | ||
I agree to storage and processing of my personal data as described in the{" "} | ||
<a target="_blank" href="/privacy" rel="noopener noreferrer"> | ||
privacy statement | ||
</a> | ||
. | ||
</Checkbox> | ||
</FormItem> | ||
)} | ||
{terms != null && !terms.enabled ? null : ( | ||
knollengewaechs marked this conversation as resolved.
Show resolved
Hide resolved
|
||
<FormItem | ||
name="tos_check" | ||
valuePropName="checked" | ||
rules={[ | ||
{ | ||
validator: (_, value) => | ||
value | ||
? Promise.resolve() | ||
: Promise.reject(new Error(messages["auth.tos_check_required"])), | ||
}, | ||
]} | ||
> | ||
<Checkbox disabled={terms == null}> | ||
I agree to the{" "} | ||
{terms == null ? ( | ||
"terms of service" | ||
) : ( | ||
<a target="_blank" href={terms.url} rel="noopener noreferrer"> | ||
terms of service | ||
</a> | ||
)} | ||
. | ||
</Checkbox> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add loading state for terms of service checkbox The checkbox is disabled when terms are loading, but there's no visual indication to the user. Consider adding a loading state: -<Checkbox disabled={terms == null}>
+<Checkbox disabled={terms == null}>
+ {terms == null ? (
+ <span>
+ <LoadingOutlined spin /> Loading terms of service...
+ </span>
+ ) : (
I agree to the{" "}
<a target="_blank" href={terms.url} rel="noopener noreferrer">
terms of service
</a>
.
+ )}
</Checkbox>
|
||
</FormItem> | ||
)} | ||
</div> | ||
|
||
<FormItem> | ||
<Button | ||
size="large" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While being nitpicky 🙈
Could you please add an error message like
? 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Else some kind of generic 400 error would be returned where an explanation would be useful IMO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can do this of course, no problem. Note that the user would not see 400 anyway because the frontend does not allow not accepting TOS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I just thought in case the logic changes in the frontend and a little bug or so is introduced, that the backend 1. ensures that tos is accepted and 2. in case it is not, the error returned is understandable.
Thanks a lot 🙏