-
Notifications
You must be signed in to change notification settings - Fork 0
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] Show hidden custom inputs only to authenticated sessions #90
Conversation
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.
LGTM
We wanted to only hide the inputs of type "hidden" no? |
Ah thanks, understood now what you meant for type hidden, will update it |
apps/web/pages/booking/[uid].tsx
Outdated
const showHiddenInputs = | ||
eventTypeCustomFound?.type === "HIDDEN" && sessionStatus === "authenticated"; | ||
if (!showHiddenInputs) { | ||
return null; | ||
} |
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.
Wait, now you will only see hidden inputs when you are logged in and nothing else. Users that are not looked in will not see anything.
Shouldn't it be like
if (eventTypeCustomFound?.type === "HIDDEN" && sessionStatus !== "authenticated") {
return null;
}
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.
the 2 logics are equivalent, I do negate the result later with !showHiddenInputs
.
I agree it's a bit hard to understand due to the !
, but hideHiddenInputs was also weird naming.
I can remove the variable and do an inline check in the if statement if you prefer
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.
The conditions are not equivalent. Imagine type
is "TEXT". Then showHiddenInputs
is false
and you will not render anything.
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.
Ah wait.
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.
It's Monday. My head works slow. But yes definitely if you just inline it without leading negation, it's easier to understand.
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.
if type is TEXT showHiddenInputs
would be true
and !showHiddenInputs
evaluates as false
Let's also add this change to the "Notificable Changes" section of the Confluence page. |
Context/Change
Show custom inputs only to authenticated users, to prevent this scenario