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

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

Closed
mathetos opened this issue Oct 26, 2017 · 9 comments

Comments

Projects
None yet
7 participants
@mathetos
Copy link
Member

commented Oct 26, 2017

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 DevinWalker added this to the 2.0.1 milestone Oct 27, 2017

@ravinderk ravinderk modified the milestones: 2.0.1, 2.0.2 Jan 22, 2018

raftaar1191 added a commit to raftaar1191/Give that referenced this issue Jan 22, 2018

raftaar1191 added a commit to raftaar1191/Give that referenced this issue Jan 24, 2018

@ravinderk ravinderk added the has pr label Jan 29, 2018

raftaar1191 added a commit to raftaar1191/Give that referenced this issue Jan 29, 2018

@DevinWalker DevinWalker modified the milestones: 2.0.2, 2.0.3 Jan 30, 2018

raftaar1191 added a commit to raftaar1191/Give that referenced this issue Jan 30, 2018

ravinderk added a commit that referenced this issue Jan 31, 2018

Merge pull request #2691 from raftaar1191/issues-2250
Add/Update Donation goal meta on donation form update or when donating #2250

ravinderk added a commit that referenced this issue Jan 31, 2018

ravinderk added a commit that referenced this issue Jan 31, 2018

Merge pull request #2750 from WordImpress/revert-2691-issues-2250
Revert "Add/Update Donation goal meta on donation form update or when donating #2250"

raftaar1191 added a commit to raftaar1191/Give that referenced this issue Jan 31, 2018

@raftaar1191 raftaar1191 referenced this issue Jan 31, 2018

Closed

Fix for #2250 #2751

3 of 3 tasks complete

@DevinWalker DevinWalker reopened this Jan 31, 2018

DevinWalker added a commit that referenced this issue Jan 31, 2018

Merge branch 'release/2.0.3' into release/2.1
* release/2.0.3: (62 commits)
  Ran gulp
  Added change log for 2.0.2 and bumped version
  Added give_v201_create_tables() to 2.0.1 upgrades just to be super sure tables are created properly.
  Use direct sql queries for upgrade instead of WP_Query
  Remove code repetation #2737
  Fix price level #2737
  Fix coding standard #2737
  Remove meta update when form is alredy being created #2737
  Add give_get_total_post_type_count fn pass2
  Add give_get_total_post_type_count fn
  Revert "Add/Update Donation goal meta on donation form update or when donating #2250"
  Use get_posts instead of WP_Query in updates
  Add helper fn give_get_total_post_type_count
  Change fn order in file give_v202_add_form_goal_meta_callback
  Reverted links to only published status because it was causing link to go to blank entry when donor had only abandoned or pending donations
  Add constructor parameter #2744
  Add constructor to make is_writable true for import #2744
  Run upgrade after 2.0.1 upgrade removed from option
  Rerun  2.0.1 update into 2.0.2
  Change delete donor key#2744
  ...

@ravinderk ravinderk added this to the 2.0.6 milestone Feb 22, 2018

@ravinderk ravinderk modified the milestones: 2.0.6, 2.0.7 Mar 5, 2018

@DevinWalker

This comment has been minimized.

Copy link
Member

commented Mar 6, 2018

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?

@DevinWalker DevinWalker removed this from the 2.0.7 milestone Mar 6, 2018

@Benunc

This comment has been minimized.

Copy link
Member

commented Apr 9, 2018

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

This comment has been minimized.

Copy link
Member

commented Apr 9, 2018

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 kevinwhoffman changed the title Add post_meta to indicate when a form is closed feat(form): allow forms to be queried based on goal status Apr 9, 2018

@kevinwhoffman kevinwhoffman added this to the Sprint: 2018/03/28 - 2018/04/10 milestone Apr 9, 2018

@DevinWalker DevinWalker changed the title feat(form): allow forms to be queried based on goal status feat(form): allow forms to be queried based on close status Apr 9, 2018

@impress-org impress-org deleted a comment from raftaar1191 Apr 9, 2018

@kevinwhoffman

This comment has been minimized.

Copy link
Member

commented Apr 9, 2018

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

This comment has been minimized.

Copy link
Contributor

commented Apr 10, 2018

@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

This comment has been minimized.

Copy link
Member

commented Apr 10, 2018

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

This comment has been minimized.

Copy link
Collaborator

commented Apr 10, 2018

@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

This comment has been minimized.

Copy link
Member

commented Apr 11, 2018

@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

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2018

Bummer. The custom post_status option sounded pretty great.

ravinderk added a commit that referenced this issue Apr 12, 2018

Merge pull request #3022 from mehul0810/issue/2250
feat(form): allow forms to be queried based on close status #2250
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.