impress-org/givewp

feat(form): allow forms to be queried based on close status

Closed this issue · 9 comments

mathetos commented

Issue Overview

Occasionally, users want to have an archive of Give forms, but exclude those that are closed because they achieved their goal. We currently have post_meta called _give_close_form_when_goal_achieved which returns just enabled|disabled, but when the goal is actually achieved and the form is closed, there's no way to query and exclude those forms. Adding _give_form_is_closed with true|false would benefit these users greatly.

DevinWalker commented

Rethinking this one through... wouldn't it be better to have function like: give_is_form_closed( $form_id ) which checks:

  1. If goal is enabled
  2. If the option to close a form upon reaching goal is set
  3. If goal the goal has been reached.

Then returns true. If false, it returns false. The downside of this would be multiple queries compared to a single meta query. What do you think?

Benunc commented

There are still users waiting for a fix for this issue, so I'm labeling it in the hopes that Devin's comment on the 6th of March led to another issue that actually resolves this.

kevinwhoffman commented

I'm putting this in 2.1 as I think it will become more requested when we introduce the form grid shortcode.

Rethinking this one through... wouldn't it be better to have function like: give_is_form_closed( $form_id ) which checks:

If goal is enabled
If the option to close a form upon reaching goal is set
If goal the goal has been reached.

@DevinWalker We already have is_close_donation_form() which does what you describe, but I don't think we want to run that against every form to determine if it is open or not. It also wouldn't work correctly with pagination if we are running checks after the query.

Recommended Solution

What we need is a way to query only forms that are published and open. A more performant solution that will work with pagination is to add a custom post status called Published (Closed) which is assigned once a form meets its goal.

image

image

We can hook into the completed donation and check to see if the form should be closed at that point. That way the calculation occurs after the donation rather than during each query in the archive.

[give_form_grid] attribute

If we like the custom status approach, we should also add a status attribute to the [give_form_grid] shortcode so that only open forms can be displayed in the grid.

kevinwhoffman commented

Call Summary

Participants: @DevinWalker @mathetos @kevinwhoffman
Topic: Best way to query forms by closed status
Result: We agree to move forward with the recommend solution in #2250 (comment).

mehul0810 commented

@DevinWalker @kevinwhoffman @ravinderk

I've researched on this and found that currently there is no way to add custom post status to Publish meta box. I've found 8 years old open Trac ticket which is still under Future Release label.

Also, I have found an alternative solution to implement the custom post status to Publish meta box using inline scripts. Refer: http://jamescollings.co.uk/blog/wordpress-create-custom-post-status/
Using this method will increase page load time of all the post detail page due to the less performant code.

I would like to suggest to proceed with the way @DevinWalker suggested in comment which will be used when required.

Let me know your thoughts on this.

kevinwhoffman commented

I think it is fine if the publish_closed status does not appear in the Publish meta box. After all, we do not want users manually changing this status anyways; we only want it changed when a goal is met and set to close. This status doesn't need to be visible to the end user. It just needs to be set in the event that a user wants to exclude forms that are closed from their query.

The suggestion in #2250 (comment) does not address the need to query forms by closed status. It only provides the functionality to check if an individual form is closed.

If we want to avoid the post status solution, then the alternative would be post meta or hidden taxonomy, but post status is the simplest query from a MySQL perspective.

ravinderk commented

@kevinwhoffman yes post_status will be simplest but it will increase complexity at the core.

I think if we will use post_meta logic then it will be clean. Since we are using custom meta table, so performance will not be a big issue. On basis of custom meta, we can show custom post status on form edit screen which will inform admin about form status.

@kevinwhoffman let me know what do you think

kevinwhoffman commented

@ravinderk @mehul0810 Let's go with the post meta approach. Seems like the path of least resistance and will get the job done.

I recommend we use _give_form_status as the post meta key with a value of closed if the form is closed. This will leave flexibility if we have other form statuses in the future.

Updated tasks below:

  • Hook into completed donations to check if a goal is enabled, has been set to close when completed, and is complete. If so, then set _give_form_status to closed.
  • Write an upgrade routine for 2.1 that loops through all forms and sets the post meta if closed.
  • Add status attribute to the [give_form_grid] shortcode. By default, the shortcode should display all published forms, but if [give_form_grid status="open"] is used, then a meta query should be added to excludes closed forms.
mathetos commented

Bummer. The custom post_status option sounded pretty great.