INN / umbrella-inndev

Umbrella repository for inn.org
GNU General Public License v2.0
0 stars 1 forks source link

Restrict access to pages based on cookies/form submissions #162

Open joshdarby opened 4 years ago

joshdarby commented 4 years ago

Changes

This pull request makes the following changes:

Notes:

Restrict access metabox: Screen Shot 2020-05-12 at 1 54 57 PM

Top level page with form: ezgif-3-3ecde6edf709

Page with no matching form but that is restricted: Screen Shot 2020-05-12 at 1 54 37 PM

Why

For #161

Testing/Questions

Features that this PR affects:

Questions that need to be answered before merging:

Steps to test this PR:

  1. Create a new page or pick a page to test with
  2. Enable the "Restrict Access" checkbox
  3. Create a new GF form
    • make the title of the form the exact title of your test page
    • make the last field of the form a hidden field and give it the value of your test page and make the label access_ID Screen Shot 2020-05-12 at 2 06 23 PM
  4. Add a child page to your test page and restrict it.
  5. Add a child page to your child page and restrict it.
  6. Open your test page in an incognito window and fill out the form. Hopefully you're redirected to the page with the content now and are able to also view the child pages.
benlk commented 4 years ago

Two implementation-detail questions:

joshdarby commented 4 years ago

Two implementation-detail questions:

  • How does this work if WPE has cached the page HTML? Does WPE in production environments serve different page results with and without the cookie?

That's a great question that I'm not sure about until we put it up on a WPE environment and test it out. We may possibly need to add some cachebusting into it.

  • Is it more future-proof if the form name is the ID of the post instead of the name of the post? That way, folks could rename the post without breaking the form.

I'm torn between yes and no here. Yes, the ID would be better in a lot of ways. But then we'll have users looking through x amount of forms trying to find the correct one because they don't know the actual ID of the page/post form they're looking for, and I'm not sure if it's a reasonable ask to expect them to know the ID.

benlk commented 4 years ago

But then we'll have users looking through x amount of forms trying to find the correct one because they don't know the actual ID of the page/post form they're looking for, and I'm not sure if it's a reasonable ask to expect them to know the ID.

We could add to the meta box a link to the appropriate form. In pseudocode:

$form_exists = false;
for post_id in array( this post ID, ancestor post IDs ) {
    if form_exists( $post_id ) {
        $form_exists = true;
        "This post's visibility would be controlled by <this form>"
    }
}
if ( ! $form_exists ) {
   "no form exists for this post. [create one](/wp-admin/admin.php?page=gf_new_form) with the title $post_id"
}

There doesn't appear to be a way to prefill the form title in the create-new-form form, unfortunately. I was hoping &new_form_title=12345 or &title=12345 would prefill it with "12345", but the plugin contains no logic to do that:

https://github.com/wp-premium/gravityforms/blob/ca668e0dc28bc42a8634be619837bbdc8f445d16/form_list.php#L96

joshdarby commented 4 years ago

But then we'll have users looking through x amount of forms trying to find the correct one because they don't know the actual ID of the page/post form they're looking for, and I'm not sure if it's a reasonable ask to expect them to know the ID.

We could add to the meta box a link to the appropriate form. In pseudocode:

$form_exists = false;
for post_id in array( this post ID, ancestor post IDs ) {
    if form_exists( $post_id ) {
        $form_exists = true;
        "This post's visibility would be controlled by <this form>"
    }
}
if ( ! $form_exists ) {
   "no form exists for this post. [create one](/wp-admin/admin.php?page=gf_new_form) with the title $post_id"
}

There doesn't appear to be a way to prefill the form title in the create-new-form form, unfortunately. I was hoping &new_form_title=12345 or &title=12345 would prefill it with "12345", but the plugin contains no logic to do that:

https://github.com/wp-premium/gravityforms/blob/ca668e0dc28bc42a8634be619837bbdc8f445d16/form_list.php#L96

@benlk I like this idea, but the inability to populate the form title with the post ID makes me weary because then we're back at the question of whether or not its a reasonable ask to expect the user to know how to find the post ID :/