[D7] When Batch ID doesn't exist, Drupal should emit a 404 [#2826866]

Follow-up to #1986330: When Batch ID doesn't exist, Drupal should emit a 404

Problem/Motivation

Currently if you try to go to /batch?id=123 you get redirected to the homepage with a message stating "No active batch".

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category	Usability Bug, because the message "No active batch" is not clear, since no batch is ran and it does not exists. Response must help figure out what is going on actually.
Issue priority	Normal, because this is a self-contained bug and does not impact the overall functionality of the software.
Unfrozen changes	Not unfrozen because it changes automated tests and needs a bug fix.
Prioritized changes	Prioritized, the main goal of this issue is a bug fix, usability and user experience improvement.
Disruption	Impact on improving usability is fair. Also it doesn't disrupt core API because it is internal to batch API only.

Proposed resolution

Since the batch doesn't exist Drupal should just emit a 404 instead

User interface changes

N/A

API changes

If you try to go to /batch?id=123 Drupal emit a 404.

Comment	File	Size	Author
#19	d7_when_batch_id-2826866-19.patch	520 bytes	cebasqueira
#19	7.x: PHP 5.3 & MySQL 5.5 2,030 pass PHP 5.6 & MySQL 5.5 2,030 pass PHP 7 & MySQL 5.5 2,030 pass
#14	Screen Shot 2017-01-12 at 12.23.55 PM.png	112.91 KB	nikunjkotecha
#10	2826866-10.patch	519 bytes	nikunjkotecha
#10	7.x: PHP 5.3 & MySQL 5.5 2,030 pass PHP 5.4 & MySQL 5.5 2,030 pass PHP 5.4 & MySQL 5.5 2,030 pass PHP 7 & MySQL 5.5 2,030 pass
#6	d7_when_batch_id-2826866-6.patch	585 bytes	lomasr
#6	7.x: PHP 5.3 & MySQL 5.5 2,030 pass PHP 5.4 & MySQL 5.5 2,030 pass PHP 7 & MySQL 5.5 2,030 pass
#3	interdiff-d7_when_batch_id-2826866-3.txt	532 bytes	Yago Elias
#3	d7_when_batch_id-2826866-3.patch	677 bytes	Yago Elias
#3	7.x: PHP 5.3 & MySQL 5.5 Patch failed to apply
#2	2826866-2.patch	401 bytes	nikunjkotecha
#2	7.x: PHP 5.3 & MySQL 5.5 2,025 pass

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

13 November 2016 at 07:24

nikunjkotecha created an issue. See original summary.

Comment #3

Yago Elias CreditAttribution: Yago Elias as a volunteer and at CI&T commented 9 December 2016 at 16:59

Status:

Needs review

» Needs work

File	Size
d7_when_batch_id-2826866-3.patch	677 bytes
7.x: PHP 5.3 & MySQL 5.5 Patch failed to apply
interdiff-d7_when_batch_id-2826866-3.txt	532 bytes

We should add a message log to the dblog system.

Comment #4

Yago Elias CreditAttribution: Yago Elias as a volunteer and at CI&T commented 9 December 2016 at 18:53

Status:

Needs work

» Needs review

Comment #5

9 December 2016 at 18:56

Status:

Needs review

» Needs work

The last submitted patch, 3: d7_when_batch_id-2826866-3.patch, failed testing.

Comment #6

lomasr CreditAttribution: lomasr at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commented 10 December 2016 at 07:31

File	Size
d7_when_batch_id-2826866-6.patch	585 bytes
7.x: PHP 5.3 & MySQL 5.5 2,030 pass PHP 5.4 & MySQL 5.5 2,030 pass PHP 7 & MySQL 5.5 2,030 pass

Adding a patch for Text change.Please review.

Comment #7

lomasr CreditAttribution: lomasr at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commented 10 December 2016 at 07:31

Status:

Needs work

» Needs review

Comment #8

10 December 2016 at 07:43

Status:

Needs review

» Needs work

The last submitted patch, 6: d7_when_batch_id-2826866-6.patch, failed testing.

Comment #9

nikunjkotecha

he/him

Gujarati

India, Gujarat, Rajkot

CreditAttribution: nikunjkotecha as a volunteer commented 15 December 2016 at 05:20

Assigned:

Unassigned

» nikunjkotecha

Comment #10

nikunjkotecha

he/him

Gujarati

India, Gujarat, Rajkot

CreditAttribution: nikunjkotecha as a volunteer commented 11 January 2017 at 11:35

File	Size
2826866-10.patch	519 bytes
7.x: PHP 5.3 & MySQL 5.5 2,030 pass PHP 5.4 & MySQL 5.5 2,030 pass PHP 5.4 & MySQL 5.5 2,030 pass PHP 7 & MySQL 5.5 2,030 pass

4 files were hidden/shown/deleted

File	Size
2826866-2.patch	401 bytes
7.x: PHP 5.3 & MySQL 5.5 2,025 pass
d7_when_batch_id-2826866-3.patch	677 bytes
7.x: PHP 5.3 & MySQL 5.5 Patch failed to apply
interdiff-d7_when_batch_id-2826866-3.txt	532 bytes
d7_when_batch_id-2826866-6.patch	585 bytes
7.x: PHP 5.3 & MySQL 5.5 2,030 pass PHP 5.4 & MySQL 5.5 2,030 pass PHP 7 & MySQL 5.5 2,030 pass

Updated patch to include the message before drupal_exit().

Comment #11

nikunjkotecha

he/him

Gujarati

India, Gujarat, Rajkot

CreditAttribution: nikunjkotecha as a volunteer commented 11 January 2017 at 11:35

Status:

Needs work

» Needs review

Comment #12

11 January 2017 at 12:19

Status:

Needs review

» Needs work

The last submitted patch, 10: 2826866-10.patch, failed testing.

Comment #13

nikunjkotecha

he/him

Gujarati

India, Gujarat, Rajkot

CreditAttribution: nikunjkotecha as a volunteer and at Axelerant commented 12 January 2017 at 06:52

Status:

Needs work

» Needs review

I'm not sure what's the error for fail as it shows nothing in error and it passed in simplytest.me for me.

File	Size
Screen Shot 2017-01-12 at 12.23.55 PM.png	112.91 KB

Comment #15

13 January 2017 at 05:48

Status:

Needs review

» Needs work

The last submitted patch, 10: 2826866-10.patch, failed testing.

Comment #16

AjitS

he/him

Marathi

Pune

CreditAttribution: AjitS as a volunteer and at QED42 commented 13 January 2017 at 06:33

+++ b/includes/batch.inc
@@ -51,8 +51,9 @@ function _batch_page() {
+      watchdog('Batch', '@batch_id is not a valid batch id.', array('@batch_id' => $_REQUEST['id']), WATCHDOG_ERROR);

Can we get a better message? This sounds very technical.
How about

Trying to execute an invalid batch. ID : @batch_id

Comment #17

navneet0693 CreditAttribution: navneet0693 as a volunteer and at QED42 commented 13 January 2017 at 07:03

Agreed with comment in #16

Comment #18

navneet0693 CreditAttribution: navneet0693 as a volunteer and at QED42 commented 13 January 2017 at 09:00

+++ b/includes/batch.inc
@@ -51,8 +51,9 @@ function _batch_page() {
+      watchdog('Batch', '@batch_id is not a valid batch id.', array('@batch_id' => $_REQUEST['id']), WATCHDOG_ERROR);

It's just a suggest if we use % instead of @

Comment #19

cebasqueira CreditAttribution: cebasqueira at CI&T commented 16 January 2017 at 15:41

Status:

Needs work

» Needs review

File	Size
d7_when_batch_id-2826866-19.patch	520 bytes
7.x: PHP 5.3 & MySQL 5.5 2,030 pass PHP 5.6 & MySQL 5.5 2,030 pass PHP 7 & MySQL 5.5 2,030 pass

Needs review

» Reviewed & tested by the community

Comment #23

stefan.r CreditAttribution: stefan.r commented 26 January 2017 at 22:34

How does this work in Drupal 8? If it has the same issue, this should be fixed there first.

Comment #24

David_Rothstein CreditAttribution: David_Rothstein as a volunteer commented 28 January 2017 at 05:31

Status:

Reviewed & tested by the community

» Needs work

The Drupal 8 issue is the parent of this one (#1986330: When Batch ID doesn't exist, Drupal should emit a 404). Looks like it was just marked RTBC, but the two patches are not in sync. They use different wording, and (especially) this one is missing the automated test.