Avoid duplicate notifications

[nkaretnikov]

This is an update of https://gitorious.org/snowdrift/snowdrift/merge_requests/121.

[nkaretnikov]

@singpolyma But it's less explicit, which is not desirable in this case, in my opinion.

[nkaretnikov]

This shouldn't be merged until I write a proper migration file (the current one just drops the column, but it's necessary to move some entries to the project table) and rebase.

[singpolyma]

@nkaretnikov would help if you reply in context :)

sortBy compare reads as an unidiomatic bug to me, not as "more explicit". Of course you sort by compare! What else would you sort by? (Other than something else you might specify, but I mean, compare is the obvious comparison function. That's it's whole purpose to exist at all.) I mean, compare itself is polymorphic, so specifying compare doesn't actually specify anything, IMHO.

[wolftune]

FWIW, @singpolyma makes sense to me, sortBy compare gives no information over just sort

[chreekat]

sort is defined as 'sortBy compare' in both Data.List and Data.Sequence. If you ever need to write 'sortBy compare', you should just write 'sort'.

On Tue, Apr 14, 2015 at 08:57:24AM -0700, Aaron Wolf wrote:

FWIW, @singpolyma makes sense to me, sortBy compare gives no information over just sort

— Reply to this email directly or view it on GitHub.*

[nkaretnikov]

Please don't merge yet! migrations/migrate63 needs to be rewritten to account for 4585299 and 7ac1100.

[nkaretnikov]

Okay, I think it's ready for merge. Please review.

[chreekat]

There seems to be a bug. When I reply to a comment, Snowdrift prints this to stdout:

Snowdrift: PersistMarshalError "Invalid UserNotificationDelivery: NotifDeliverEmail"

NotifDeliverEmail changes depending on the "to" user's preferences.

No notifications are created or displayed, either.

[nkaretnikov]

I wonder why it worked when I tested it, but, yeah, it should beUserNotifDeliverEmail in this case.

[chreekat]

By the way, this is what I added to the migrate file to remove existing duplicate notifications:

-- First, clear existing duplicate notifications.
-- (see https://wiki.postgresql.org/wiki/Deleting_duplicates).
DELETE FROM "notification"
WHERE "id" IN (
    SELECT "id"
    FROM (
        SELECT "id", row_number() OVER win AS rnum
        FROM "notification"
        WINDOW win AS (
            PARTITION BY "created_ts", "type", "to"
        )
    ) subq
    WHERE subq.rnum > 1
);

[chreekat]

Merged.