I've noticed that the ads content that is crea.ted updates constantly. When I go to the content menu they are always on top. Maybe this has something to do with cron? Normally it's not an issue, only making it annoying to search for content because I have to scroll down all the time. However, when we have a few users posting, editing and such, this seems to cause unnecessary load. I don't believe it should be updating these all the time. Is there a way to make this not happen without hacking the code? Is anyone else getting this issue? Any help is much appreciated

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

minnur’s picture

I can confirm this. Right now I don't have time to fix this, I will try get back to this when I have some capacity.

Ganginator’s picture

It's good to hear that it's not just me.
Is there anything I can do to help?

Bcwald’s picture

I was just going to create an issue for this. It doesnt seem like a big deal, but on my magazine, we have many articles that go live each day, so its nice to see an accurate recent posting.

I will look into the code to see if I can figure anything out.

Bcwald’s picture

Anyone have any idea what is causing this to be updated, i would love to get this fixed as its taking over the content management area.

Ganginator’s picture

I'm pretty sure it's updating all simpleads content on every cron run, and that this is maybe because of it's tracking system updating clickthroughs.

mavdiablo’s picture

Hi there,
you are right when you say that all simpleads content are updated on every cron run.
But not for tracking system, for status system.
I mean that every cron the script checks the start date and the end date of simpleads content and set the status to 0 if the simpleads is expired or 1 if simpleads is actived.
But I think it is useless to save the node if the state does not change.
So the site spares even a few seconds of the cron procedure.
Here what I mean:
File simpleads.helper.inc
Function _simpleads_activate_deactive_ad

  $old_status=$node->status;
  
  if ($start_time != '' && $end_time != '') {
    if ($now >= $start_time && $end_time > $now) {
      $node->status = 1;
      if ($who == 'cron' && $node->status!=$old_status) {
        node_save($node);
      }
    }
    elseif ($end_time <= $now) {
      $node->status = 0;
      if ($who == 'cron' && $node->status!=$old_status) {
        node_save($node);
      }
    }
  }
  elseif ($start_time == '' && $end_time != '') {
    if ($end_time <= $now) {
      $node->status = 0;
      if ($who == 'cron' && $node->status!=$old_status) {
        node_save($node);
      }
    }
    else {
      $node->status = 1;
      if ($who == 'cron' && $node->status!=$old_status) {
        node_save($node);
      }
    }
  }
  else {
    if ($now >= $start_time && $start_time != '') {
      $node->status = 1;
      if ($who == 'cron' && $node->status!=$old_status) {
        node_save($node);
      }
    }
  }

I hope it will useful for you
Bye

Bcwald’s picture

Status: Active » Needs review

I tried out the solution in #6 and seems to work. When I ran cron, it did not update any of the items that didnt need updating, but when something expired, on next cron run, it shows up in the list.

mavdiablo’s picture

yes, that's right... because the node has been saved and I think it's good to see it updated as you realize that something has happened.
Is it not right for you?

Bcwald’s picture

I have created a patch for this (based off the latest dev version)

Status: Needs review » Needs work

The last submitted patch, 7.x-1.x-dev-fix-for-ad-update.patch, failed testing.

mavdiablo’s picture

Status: Needs work » Needs review

thanks :)

Ganginator’s picture

I like that we're making progress!
I completely understand, and support simple ads updating after something expires, or is edited, and therefore appearing on the top of the content list.
That is perfect.
It just needs to not update all ads on every cron.
If the state does not change, do not update, if it has, update.

minnur’s picture

Hi there, I think I fixed the problem, please try development version of SimpleAds and report any problems here.

I would not recommend to install it on your production environment. Please use stage or local dev environment instead.

This release also contains fix for this http://drupal.org/node/1876732

Thanks

mahaprasad’s picture

FileSize
3.72 KB

Issue: On every cron run, all simpleads nodes are fetched from node table & in function _simpleads_activate_deactive_ad(), all these simpleads nodes are updated as per added ads date.

Now to solve this problem I have created queries to load only those simpleads nodes which are going to Activate or De-active based on the date added & updated these nodes.

Please review the attached patch.

Thank you!

Status: Needs review » Needs work

The last submitted patch, cronfix-1853466-14.patch, failed testing.

mahaprasad’s picture

FileSize
5.09 KB

Corrected the uploaded patch in #14.
Please review.

mahaprasad’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, cronfix-1853466-16.patch, failed testing.

Tran’s picture

What's the status of this issue?

jenniferblair’s picture

Issue summary: View changes

I'm looking for a status update on this issue as well.

oo7_golden_1’s picture

I am interested in a status update as well... Love the module by the way.

VM’s picture

Issue summary: View changes

The status based off of the dev's comment in #13 is that -dev should be tested. This remains true today as the date of the last release is earlier than the date of the comment.

minnur’s picture

Status: Needs work » Closed (won't fix)