Note: This issue is reserved for an intern/applicant at MD Systems. Please do not work on this unless assigned to it by us :)

Problem/Motivation

When using the Export form at admin/people/simplenews/export and checking both "Active" and "Inactive", there is an SQL error.

Proposed resolution

Fix the status condition in SubscriberExportForm.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#13 sql_error_export_form-2772717-13.patch1.87 KBGinovski
#13 sql_error_export_form-2772717-13-test-only.patch1.17 KBGinovski
#11 sql_error_export_form-2772717-11.patch1.68 KBGinovski
#11 interdiff-2772717-10-11.txt488 bytesGinovski
#10 sql-error-export-form-2772717-10.patch1.68 KBGinovski
#10 interdiff-2772717-5-10.txt1004 bytesGinovski
#5 sql-error-export-form-2772717-5.patch716 bytesGinovski
#3 sql-error-export-form-2772717-3.patch716 bytesGinovski
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Arla created an issue. See original summary.

Arla’s picture

Arla
they/them
CreditAttribution: Arla at MD Systems GmbH commented
Title: SQL error for status field on export form » SQL error on export form
Issue summary: View changes

Same goes for the "Subscribed" selection.

Ginovski’s picture

Ginovski CreditAttribution: Ginovski at MD Systems GmbH commented
Assigned: Unassigned » Ginovski
Status: Active » Needs review
FileSize
sql-error-export-form-2772717-3.patch716 bytes
Arla’s picture

Arla
they/them
CreditAttribution: Arla at MD Systems GmbH commented

Looks good to me. The test won't run because of a branch fail: #2789215: Fix failing tests Let's get that fixed so we can commit this then.

Ginovski’s picture

Ginovski CreditAttribution: Ginovski at MD Systems GmbH commented
FileSize
sql-error-export-form-2772717-5.patch716 bytes

Updating with fixed tests.

Status: Needs review » Needs work

The last submitted patch, 5: sql-error-export-form-2772717-5.patch, failed testing.

The last submitted patch, 5: sql-error-export-form-2772717-5.patch, failed testing.

Ginovski’s picture

Ginovski CreditAttribution: Ginovski at MD Systems GmbH commented
Status: Needs work » Needs review
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Where are the tests? :)

Ginovski’s picture

Ginovski CreditAttribution: Ginovski at MD Systems GmbH commented
Status: Needs work » Needs review
FileSize
interdiff-2772717-5-10.txt1004 bytes
sql-error-export-form-2772717-10.patch1.68 KB

With added tests now

Ginovski’s picture

Ginovski CreditAttribution: Ginovski at MD Systems GmbH commented
FileSize
interdiff-2772717-10-11.txt488 bytes
sql_error_export_form-2772717-11.patch1.68 KB

Removed extra line

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Needs work

We have existing tests for the export. Lets extend those instead of adding more test methods. Also, always upload a test-only patch when writing test coverage for a bugfix.

Ginovski’s picture

Ginovski CreditAttribution: Ginovski at MD Systems GmbH commented
Status: Needs work » Needs review
FileSize
sql_error_export_form-2772717-13-test-only.patch1.17 KB
sql_error_export_form-2772717-13.patch1.87 KB

Configured test function

The last submitted patch, 13: sql_error_export_form-2772717-13-test-only.patch, failed testing.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Fixed

Now I like it :)

Committed.

  • Berdir committed 41a9e5c on 8.x-1.x authored by Ginovski 
    Issue #2772717 by Ginovski: SQL error on export form

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.