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.

Sign up for GitHub

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

Avoid duplicate POST on post-submit quiz page reload #4123

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open

Avoid duplicate POST on post-submit quiz page reload #4123

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions includes/class-sensei-quiz.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ public function __construct( $file = __FILE__ ) {
add_action( 'template_redirect', array( $this, 'reset_button_click_listener' ) );

// Fire the complete quiz button submit for grading action.
add_action( 'sensei_single_quiz_content_inside_before', array( $this, 'user_quiz_submit_listener' ) );
add_action( 'template_redirect', array( $this, 'user_quiz_submit_listener' ) );
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we do this without changing the action? This would be a breaking change for any customizations that remove or modify these hooks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, changing the action to what's better aligned with the callbacks' purpose is sort of the whole point here.. 🤔

There's also prior art right around these add_action calls that use template_redirect, so we also achieve better systemic alignment here.

Breaking change: yes - what is Sensei's policy for versioning? Such is usually handled by new major versions, but I'm aware WP and WC don't follow semver.

It seems like this change is fairly well "updatable" in customizations, if we highlight it well enough in release documentation.

Your thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

We have this deprecation policy that we try to adhere to, but it doesn't cover changing actions like this 🤔

I was thinking that we might be able to fix the problem (refresh causing quiz submit) without changing the hook, but I agree that moving the functionality out of the rendering actions ultimately makes more sense.

Let me circle back on this.


// Fire the save user answers quiz button click responder.
add_action( 'sensei_single_quiz_content_inside_before', array( $this, 'user_save_quiz_answers_listener' ) );
add_action( 'template_redirect', array( $this, 'user_save_quiz_answers_listener' ) );

// Fire the load global data function.
add_action( 'sensei_single_quiz_content_inside_before', array( $this, 'load_global_quiz_data' ), 80 );
Expand Down Expand Up @@ -179,6 +179,9 @@ public function user_save_quiz_answers_listener() {
// remove the hook as it should only fire once per click
remove_action( 'sensei_single_quiz_content_inside_before', 'user_save_quiz_answers_listener' );

// Prevent duplicate submit on page reload.
wp_safe_redirect( get_permalink() );

} // end user_save_quiz_answers_listener

/**
Expand Down Expand Up @@ -355,6 +358,9 @@ public function user_quiz_submit_listener() {

self::submit_answers_for_grading( $quiz_answers, $_FILES, $lesson_id, $current_user->ID );

// Prevent duplicate submit on page reload.
wp_safe_redirect( get_permalink() );

} // End sensei_complete_quiz()

/**
Expand Down