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

Anonymous/walk-in registration #205

Merged
merged 21 commits into from Mar 10, 2018
Merged

Anonymous/walk-in registration #205

merged 21 commits into from Mar 10, 2018

Conversation

bunsenmcdubbs
Copy link
Contributor

@bunsenmcdubbs bunsenmcdubbs commented Mar 5, 2018

Closes #194

3 options have been added for application branches in the admin panel

  1. Allow anonymous submissions: - Enables admins to navigate to /register/<branch-name> and register new users without accounts.
  2. Auto-accept applicants - Pretty self-explanatory, any applicants on this branch will be automatically accepted. Two caveats:
    1. "Post-accept" emails will no longer be sent. This is intended behavior. Please be sure that any information in a "post-accept" email is in either post-apply or post-confirm emails (including reminding applicants that they need to return to the site to RSVP if confirmation is required).
    2. This is not retroactive, any users who submitted their applications to this branch before the option was enabled still need to be manually accepted.
  3. Skip Confirmation - indicates that there is no confirmation step for this application branch. The final stage for applicants in this stage is "accepted". (Implemented by automatically marking users as attending=true without a confirmation branch).

#207 is a follow-up that should be implemented to add full functionality for users created in the "anonymous submission" workflow.

This will enable Walken registration workflow. When all three options are enabled on an application branch.

@bunsenmcdubbs bunsenmcdubbs added enhancement Adds or suggests a new feature, rewrite, or other enhancement to the codebase high priority labels Mar 5, 2018
@hackgbot
Copy link

hackgbot commented Mar 5, 2018

Hey y'all! A deployment of this PR can be found here:
https://registration-anonymous-form-submission.pr.hack.gt

@petschekr petschekr added this to the BuildGT 2018 milestone Mar 5, 2018
@bunsenmcdubbs
Copy link
Contributor Author

bunsenmcdubbs commented Mar 7, 2018

I plan on implementing this feature in 3 parts.

  1. Implementing a "no confirmation workflow" as described in Add support for no-confirmation workflow #150. This would remove the empty "confirm" step for any accepted applicants on branches without confirmation branches. Closes Add support for no-confirmation workflow #150
    • This could be implemented either with:
      • ➡️ database write (application "confirms" automatically when no confirmation branch is present)
      • OR pure UI implementation when the user loads the page. This is more flexible because adding a confirmation step to an application branch after-the-fact would present users with prompt if they haven't confirmed yet. This is also potentially more confusing for users.
    • timeLimited middleware needs be modified to filter down only to available confirmation branches This already happens
    • Add UI to admin screen
  2. Implementing an "auto-accept" feature for branches
    • Auto-accept users that apply to enabled branches
    • Disable "you are accepted" emails for auto-accepted users
      • Remove "accepted" email for auto-accept branches in admin screen
    • Add UI to admin screen
  3. Implementing anonymous form submissions (without email confirmation)
  • Known issue/untested edge case: registering with duplicate emails. willfix/investigate later

Fixed in 04dc661
screen shot 2018-03-08 at 10 53 08 pm

@bunsenmcdubbs bunsenmcdubbs changed the title [WIP] Anonymous form submission Anonymous registration Mar 7, 2018
@bunsenmcdubbs
Copy link
Contributor Author

Rebased on top of master (fixed tiny conflict)

@bunsenmcdubbs
Copy link
Contributor Author

bunsenmcdubbs commented Mar 7, 2018

View of admin panel with new options.
screen shot 2018-03-07 at 1 57 37 am

Top of the anonymous registration view - url: localhost:3000/register/Participant and the user is current an admin user. Email is automatically added when there is no authenticated user. The "sidebar" aka top nav is also hidden when this is the case.
screen shot 2018-03-07 at 1 58 30 am

@bunsenmcdubbs bunsenmcdubbs changed the title Anonymous registration Anonymous/walk-in registration Mar 7, 2018
</div>
</form>
{{/sidebar}}
<script type="text/javascript">
formTypeString = "Application";
Copy link
Member

Choose a reason for hiding this comment

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

WTF is this a global variable?? Remove the type attribute from the <script> tag. If you need a global variable, do window.myVariable = x instead so that it will work in strict mode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

adding it to window won't work because typescript doesn't recognize additional properties on window

@@ -85,5 +85,8 @@ <h1 class="center">RSVP: {{branch}}</h1>
</div>
</form>
{{/sidebar}}
<script type="text/javascript">
formTypeString = "Confirmation";
Copy link
Member

Choose a reason for hiding this comment

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

Globals again

@@ -642,6 +645,61 @@ for (let i = 0; i < timeInputs.length; i++) {
timeInputs[i].value = moment(new Date(timeInputs[i].dataset.rawValue || "")).format("Y-MM-DDTHH:mm:00");
}

// Uncheck available confirmation branches for application branch when "skip confirmation" option is selected
function uncheckConfirmationBranches(applicationBranch: string) {
let checkboxes = document.querySelectorAll(`.branch-role[data-name="${applicationBranch}"] .availableConfirmationBranches input[type="checkbox"]`);
Copy link
Member

Choose a reason for hiding this comment

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

Types will work better here if it's let checkboxes: NodeListOf<HTMLInputElement> = x. Then you don't need the ugly cast on line 652.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Typescript won't allow me to cast NodeListOf<Element> to NodeListOf<HTMLInputElement>

(input as HTMLInputElement).checked = false;
}
}
let skipConfirmationToggles = document.querySelectorAll(".branch-role input[type=\"checkbox\"].noConfirmation");
Copy link
Member

Choose a reason for hiding this comment

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

Same thing with types

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same thing with how its not possible.

checkbox.click();
}
}
let availableConfirmationBranchCheckboxes = document.querySelectorAll(".branch-role fieldset.availableConfirmationBranches input[type=\"checkbox\"]");
Copy link
Member

Choose a reason for hiding this comment

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

Better typing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same thing with how its not possible.


// Select "skip confirmation" option when "allow anonymous" option is selected
// Hide/show public link when "allow anonymous" is clicked
let allowAnonymousCheckboxes = document.querySelectorAll(".branch-role input[type=\"checkbox\"].allowAnonymous");
Copy link
Member

Choose a reason for hiding this comment

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

Better typing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same thing with how its not possible.


// This operation is all or nothing because it will only error if a branch was just made into an Application branch
try {
branchData.allowAnonymous = (branchRoles[i].querySelector("fieldset.applicationBranchOptions input[type=\"checkbox\"].allowAnonymous") as HTMLInputElement).checked;
Copy link
Member

Choose a reason for hiding this comment

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

Can you make fieldset.applicationBranchOptions input[type="checkbox"] a variable or something somewhere? Seems like it's getting repeated a lot. Or consider running a querySelector() on that element up front and querySelector() on the returned element for better performance and readability

@@ -2,7 +2,10 @@ enum FormType {
Application,
Confirmation
}
let formType = window.location.pathname.match(/^\/apply/) ? FormType.Application : FormType.Confirmation;
Copy link
Member

Choose a reason for hiding this comment

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

Whyyyyyyyyy globals. There has to be a better way

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this crime. I literally removed this global.

window.location.assign("/");

if (unauthenticated) {
(document.querySelector("form") as HTMLFormElement).reset();
Copy link
Member

Choose a reason for hiding this comment

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

Cast not needed here. Typescript can tell that searching for "form" will always return a form element

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue is that the type returned is HTMLFormElement | null and I want to force it to be not null.

next();
}
else {
response.status(408).json({
Copy link
Member

Choose a reason for hiding this comment

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

Is a request timeout the appropriate status code for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you are a sassyboi

Copy link
Contributor Author

Choose a reason for hiding this comment

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

literally you did this

@petschekr
Copy link
Member

Still need to test and run it on my machine but overall looks good

@petschekr petschekr merged commit 8cf4601 into master Mar 10, 2018
@petschekr petschekr deleted the anonymous-form-submission branch February 25, 2019 18:44
rahulrajus pushed a commit that referenced this pull request Jul 22, 2021
Anonymous/walk-in registration

I'm sure I won't regret this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Adds or suggests a new feature, rewrite, or other enhancement to the codebase high priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants