[8381] add unregistered participants field to project insights to account fo… #2830
Conversation
goapunk
changed the title
goapunk
changed the title
goapunk
force-pushed
the
jd-2024-11-poll-insights
branch
from
8d086c5 to
3481b63
Compare
goapunk
force-pushed
the
jd-2024-11-poll-insights
branch
2 times, most recently
from
5ed20f2 to
63ad043
Compare
|
I've updated this with the signal solution to compare now |
goapunk
force-pushed
the
jd-2024-11-poll-insights
branch
from
63ad043 to
554836c
Compare
goapunk
changed the title
|
@goapunk there is an error also with the signals import, tests failing |
@m4ra will do, the signal error is because it needs the a4 pr merged first |
goapunk
force-pushed
the
jd-2024-11-poll-insights
branch
from
554836c to
1aabe86
Compare
| (_("active participants"), insight.active_participants.count()), | ||
| ( | ||
| _("active participants"), | ||
| insight.active_participants.count() + insight.unregistered_participants, |
There was a problem hiding this comment.
here active participants are the sum of both registered and unregistered. But when a poll is submitted by an unregistered user, the active participants are not increased, only the unregistered ones. See my comment below in the signal.
There was a problem hiding this comment.
active_participants is a m2m relation to the user model, so we can't increase it for unregistered users, that's why I added the unregistered_participants field. And we combine both fields here to reflect the total count of participants (maybe somewhat confusingly still called "active participants").
| if creator: | ||
| insight.active_participants.add(creator.id) | ||
| else: | ||
| insight.unregistered_participants += 1 |
There was a problem hiding this comment.
here, shouldn't we also increment the active_participants?
|
@goapunk can you update the a4 hash, and add a changelog? I have approved already, so please go ahead and merge once you are done. |
goapunk
force-pushed
the
jd-2024-11-poll-insights
branch
from
1aabe86 to
d92e768
Compare
|
@goapunk looks like the project-insight model needs to have the active_participants field as nullable. See the test failing. |
| insight.active_participants.add(*ids) | ||
| # content from unregistered users doesn't have a creator but a content_id | ||
| if model_field_exists(obj.model, "content_id"): | ||
| content_ids = set( |
There was a problem hiding this comment.
I now added more generic way of counting the unregistered participants, what do you think ? If that's fine I'd squash this commit and merge it afterwards
|
@goapunk LGTM. Go ahead with commits squashing and merge |
…r the new feature in the poll module which allows participating without account
goapunk
force-pushed
the
jd-2024-11-poll-insights
branch
from
8722bfe to
8e78892
Compare
…r the
new feature in the poll module which allows participating without account
Tasks
depends on liqd/adhocracy4#1688