Problem/Motivation

I'm writing context tests for Google Analytics module and I issue a context_delete(), but all assertNoRaw() are failing as the code still exists in the page.

context_delete() has a bug as the condition is never true. Same may be true for context_save(). I have no idea what $context->export_type is.

if (isset($context->name) && ($context->export_type & EXPORT_IN_DATABASE)) {

Maybe this should be more as nothing else makes sense to me:

if (isset($context->name) && ($context->export_type == EXPORT_IN_DATABASE)) {

Repro case:

    $this->drupalGet('');
    $this->assertRaw('ga("set", "expId", "Qp0gahJ3RAO3DJ18b0XoUQ");');
    $this->assertRaw('ga("set", "campaignName", "(direct)");');
    $this->assertRaw('ga("set", "campaignSource", "(direct)");');

    // Cleanup
    $deleted = context_delete($context);
    $this->assertTrue($deleted, 'Context "' . $context->name . '" has been deleted.');

    $this->drupalGet('');
    $this->assertNoRaw('ga("set", "expId", "Qp0gahJ3RAO3DJ18b0XoUQ");');
    $this->assertNoRaw('ga("set", "campaignName", "(direct)");');
    $this->assertNoRaw('ga("set", "campaignSource", "(direct)");');

Proposed resolution

Remaining tasks

User interface changes

None

API changes

None

CommentFileSizeAuthor
#6 Issue-2355101-Context-still-active-after-contextdele.patch5.84 KBhass
#4 Make-contextdelete-fail-if-it-fails-to-delete-the-co.patch4.83 KBhass
#1 Failing-context-test.patch825 byteshass
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hass’s picture

hass CreditAttribution: hass commented
FileSize
Failing-context-test.patch825 bytes

Here is a patch that proves this bug.

hass’s picture

hass CreditAttribution: hass commented
Issue summary: View changes
hass’s picture

hass CreditAttribution: hass commented
Issue summary: View changes
hass’s picture

hass CreditAttribution: hass commented
Status: Active » Needs review
FileSize
Make-contextdelete-fail-if-it-fails-to-delete-the-co.patch4.83 KB

That context_save() function is not logic to me as it always saves to the database, but does not set the $context->export_type = EXPORT_IN_DATABASE on newly added contexts. context_load() also does not set $context->export_type = EXPORT_IN_DATABASE.

function context_save($context) {
  $existing = context_load($context->name, TRUE);
  if ($existing && ($existing->export_type & EXPORT_IN_DATABASE)) {
    drupal_write_record('context', $context, 'name');
  }
  else {
    drupal_write_record('context', $context);
  }
  context_load(NULL, TRUE);
  context_invalidate_cache();
  return TRUE;
}

I guess it need to be changed to:

function context_save($context) {
  $existing = context_load($context->name, TRUE);
  if ($existing && ($existing->export_type & EXPORT_IN_DATABASE)) {
    drupal_write_record('context', $context, 'name');
  }
  else {
    $context->export_type = EXPORT_IN_DATABASE;
    drupal_write_record('context', $context);
  }
  context_load(NULL, TRUE);
  context_invalidate_cache();
  return TRUE;
}
hass’s picture

hass CreditAttribution: hass commented
Issue summary: View changes
hass’s picture

hass CreditAttribution: hass commented
FileSize
Issue-2355101-Context-still-active-after-contextdele.patch5.84 KB

As I do not understand the logic I'm changing it here.

hass’s picture

hass CreditAttribution: hass commented
Title: Context still active after context_delete() » context_delete() does not delete contexts
milos.kroulik’s picture

milos.kroulik CreditAttribution: milos.kroulik commented
Status: Needs review » Closed (works as designed)

I doubt this is the correct solution, since $context->export_type could have value 3 (if DB overrides code), which has no corresponding constant value. This is bitwise operator (https://secure.php.net/manual/en/language.operators.bitwise.php) and seems to work fine.