Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

rerunning payday throws off nparticipants #261

Closed
chadwhitacre opened this issue Aug 24, 2012 · 10 comments
Closed

rerunning payday throws off nparticipants #261

chadwhitacre opened this issue Aug 24, 2012 · 10 comments
Labels

Comments

@chadwhitacre
Copy link
Contributor

The paydays table accumulates nparticipants while payday is being run. If the run dies and we rerun it, we end up double-counting people. Surfaced on #204.

@zbynekwinkler
Copy link
Contributor

We do not show nparticipants anywhere now. One option might be to just drop it. Or if we decide to keep it, why wouldn't clearing to 0 in zero_out_pending (at the start of the payday) solve the problem? BTW: the same problem is with ntippers and ntips.

@chadwhitacre
Copy link
Contributor Author

We show nparticipants on the charts page. Here's what it looks like right now, after Gittip 85 (#1913):

screen shot 2014-01-16 at 5 05 14 pm

We should reset the nparticipants count for the current payday when we run it.

@chadwhitacre
Copy link
Contributor Author

Here's the fix:

update paydays set nparticipants=(select count(*) from participants where claimed_time < ts_start);

@zbynekwinkler
Copy link
Contributor

Isn't it easier to just add this query to the end() of payday than trying to find the issue about it, not being able to, creating new, finally finding it, closing the new... 😉

@chadwhitacre
Copy link
Contributor Author

That's a hacky fix, no? This updates all paydays, which is ugly to do every time, whether or not we need it. Better to fix this properly by ... whatever it would take. :-)

/me out of CPU

@zbynekwinkler
Copy link
Contributor

It is easy enough to update just the last one. And as for

whatever it would take

let me disagree. We should spend as little time as possible on such a minor
almost-bug. Just tracking this in the issue tracker is taking everyone's
"CPU time" that could be used for something more worth while. You know what
they say, choose your battles wisely.

@chadwhitacre
Copy link
Contributor Author

From #1915:

We should reset the nparticipants count for the current payday when we run it.

@chadwhitacre
Copy link
Contributor Author

I started working on this, but if I'm going to fix this in code that means writing a test. This happens infrequently enough that I think that manually repairing is still the path of least resistance. We should be looking at ★★★ and DevX ★ tickets when we browse tickets, and this is neither.

@zbynekwinkler
Copy link
Contributor

What I am trying to say is that just tracking this problem here takes more
"CPU time" than fixing it. If the need to make a test for it is the cause
why it is not fixed yet, well... I'd rather have it fixed without a test
and gone from the issue tracker so it never ever gets in the way when
everyone works with the issue tracker. But that is just IMHO.

@Changaco
Copy link
Contributor

This was fixed in the payday rewrite (#2508).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants