Problem/Motivation

When running through the user check process, the code does not correctly handle the offset field and will end up checking all users if anything is entered there.

Proposed resolution

Troubleshoot and fix it.

Remaining tasks

  1. Fix the issue
  2. Create a patch
  3. Review the patch
  4. Commit the patch

User interface changes

None.

API changes

None.

Data model changes

None.

Comments

oadaeh created an issue. See original summary.

oadaeh’s picture

Assigned: oadaeh » Unassigned
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new5.66 KB

The attached file fixes this.

oadaeh’s picture

Okay, so the user count/offset number were still not being handled correctly, so I re-worked the code more to come up with the new patch attached here.

  • I moved most of the batch pre-processing code out of the main batch process function, and into the batch set up function.
  • I pulled some code out of the batch set up function into separate functions, to make it easier to verify what is happening.
  • I added some more form/data validation checks.
  • I added more documentation to the form to explain what was happening.

There are three files:

  1. The patch file, which was generated against 7.x-2.x.
  2. An interdiff file, generated between this patch and the patch from comment #2.
  3. And another interdiff file, generated between this patch and the patch from comment #2, but excluding white space changes (interdiff-with-w-2669358-2-3.txt).
oadaeh’s picture

StatusFileSize
new12.96 KB

Darned flaky Internet connection. Trying to re-attach the interdiffs.

oadaeh’s picture

StatusFileSize
new12.5 KB

Okay, I got one, let's get the other.

oadaeh’s picture

StatusFileSize
new12.79 KB
new12.31 KB

Attached are updated files to replace those added in #3, #4, and #5 that included some unnecessary code some code form another patch.
These files do not include that extra code.

oadaeh’s picture

StatusFileSize
new11.59 KB

Adding the third file. I didn't think the second one got added in the previous comment.

aimeerae’s picture

I've reviewed the code. Good work!

1) Super minor:

+++ b/email_verify.check.inc
@@ -20,30 +20,44 @@ function email_verify_user_check_form($form, &$form_state) {
+    '#description' => t('
+      On this page, you can list all existing users for whom their email address
+      is not valid. Simply click the "Start" button to begin the process. If you
+      make changes to the users, you will need to click the "Start" or "Update"
+      buttons to see the new data.
+    '),

This seems a little odd. Text is t does not need to follow 80 characters limit but not a bit deal.

2) The queries are being built with string manipulation rather than using db_select but maybe there was some reason for that.

We'll get this tested soon.

Patrick Storey’s picture

Status: Needs review » Reviewed & tested by the community

Testing the patch in comment #6 at the /admin/people/email_verify .

I added a user with an email address that ended as .commmm to my database as user id #2.

I set the number of users to verify at 10. And the offset to 3 (so it shouldn't detect any incorrect users). Oddly it seemed to of started at the 4th user in my database.

If I set the number to 1 and the offset to 1, then it checks the 3rd user in my database (I am counting the first user as 0 assuming this is pulling from an array somewhere).

Ah, looks like there were some blocked/disabled users in the start of my database. Checked the " Include blocked/disabled users" checkbox and now the email verify is working as expected.

Okay I can verify that the offset button is working as intended. And when verifying the .commmm address it showed it in the output below as not verified with the reason being " No DNS records were found, using checkdnsrr() with "hook42.commmm." for host and "ANY" for type."

This has passed testing.

  • oadaeh committed 5a04163 on 7.x-2.x
    Issue #2669358 by oadaeh, Aimee Degnan, Patrick Storey: User check form...
oadaeh’s picture

Status: Reviewed & tested by the community » Fixed

This has been committed to the 2.x dev branch. Thanks for the reviews and testing.

oadaeh’s picture

Issue summary: View changes
kristen pol’s picture

Thanks!

oadaeh’s picture

Status: Fixed » Closed (fixed)

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