New Notifications aren't sent/received about boardrules HOT 10 CLOSED

I discovered this has to do with IDs. Since we are sending a notification for the entire rules, not individual rules with IDs, our notification's item_id is usually 0. So same ID means no new notification generated until the old one is pruned at some point.

So the question is, is this behavior acceptable? Or do we want to do something like generate a bogus unique ID, like:

public static function get_item_id($data)
{
    return !empty($data['rule_id']) ? $data['rule_id'] : rand(0, 16777215);
}

Although this may have negative consequences on the URL generated for the notification's link to the rules page.

from boardrules.

Would the unix timestamp work as the id or is that too long?

from boardrules.

Too long. Field is mediumint 8

from boardrules.

Is there no way to over-ride the method checking for duplicates? I thought there was...

from boardrules.

I have not looked deep into the notification system to know the answer to that.

from boardrules.

Should we run delete_notifications() prior to add_notifications() ?

In theory, this would delete the notification we are about to add, if it currently exists, clearing the way for a fresh new one.

from boardrules.

That could work, but I don't really like deleting history...
On Jul 1, 2014 11:16 PM, "Matt Friedman" [email protected] wrote:

Should we run delete_notifications() prior to add_notifications() ?

—
Reply to this email directly or view it on GitHub
#134 (comment)
.

from boardrules.

I have come up with two other options, but I do not know which one is the best solution:

Option 1:
We set the notification's item_id using truncated time() instead of the rule_id. In the board's current set up, we are not actually getting rule_id's because the notification is global, which is why the rule_id/item_id we get is always 0. I originally set up the board notifications to use rule_ids in case we ever decided to make the notifications work per rule.

So to do option 1, we would change:

public static function get_item_id($data)
{
    //return $data['rule_id']; // the old code
    return substr(time(), -7);
}

public function get_url()
{
    //$rule_id = ($this->item_id) ? array('#' => $this->item_id) : array(); // the old code
    $rule_id = ($this->get_data('rule_id')) ? array('#' => $this->get_data('rule_id')) : array();

    return $this->helper->route('boardrules_main_controller', $rule_id);
}

Option 2:
Instead look for users who have existing notifications that they marked as read, and reset them back to unread by adding this code to find_users_for_notification()

// Try to find the users who already have been notified about rules and just update their notifications
$sql = 'SELECT n.*
    FROM ' . $this->notifications_table . ' n, ' . $this->notification_types_table . ' nt
    WHERE n.notification_type_id = ' . (int) $this->notification_type_id . '
        AND n.item_parent_id = ' . (int) self::get_item_parent_id($data) . '
        AND n.notification_read = 1
        AND nt.notification_type_id = n.notification_type_id
        AND nt.notification_type_enabled = 1';
$result = $this->db->sql_query($sql);
while ($row = $this->db->sql_fetchrow($result))
{
    // Do not create a new notification
    unset($users[$row['user_id']]);

    $sql = 'UPDATE ' . $this->notifications_table . '
        SET notification_read = 0
        WHERE notification_id = ' . $row['notification_id'];
    $this->db->sql_query($sql);
}
$this->db->sql_freeresult($result);

I'm not really a fan of any of the options presented, but I haven't been able to find some magic method buried inside the notifications system that would solve this more elegantly. I worry about the side effects of option 2 on a board with thousands and thousands of users too.

from boardrules.

Well, the least hacky way I'd vote for is to add a counter as a config option that we just increment each time.

I checked the notifications code and there isn't any way to get around this for now...

from boardrules.

Solved by PR #138

from boardrules.