This has been broken for 3 years - is there noone that can work on this?

The issue is more substantial than just this; the poll creation/edit page doesn't allow certain changes at certain times, and not all of it seems logical. I added a parent task.

Change 665210 had a related patch set uploaded (by STran; owner: STran):
[mediawiki/extensions/SecurePoll@master] Check date validity in a validation callback

https://gerrit.wikimedia.org/r/665210

Change 665211 had a related patch set uploaded (by STran; owner: STran):
[mediawiki/extensions/SecurePoll@master] [WIP] Manually validate dynamically required fields

https://gerrit.wikimedia.org/r/665211

Change 665221 had a related patch set uploaded (by STran; owner: STran):
[mediawiki/extensions/SecurePoll@master] Clean up formData before writing it to db

https://gerrit.wikimedia.org/r/665221

Well this was (and continues to be) a fun ticket. To summarize:

https://gerrit.wikimedia.org/r/665210 Check date validity in a validation callback
tl;dr - the validation for that input was being run as a critical step in the processing function so it had to be filled out no matter what. This patch moves that validation step into a callback on the input.

https://gerrit.wikimedia.org/r/665211 Manually validate dynamically required fields
tl;dr - inputs that are considered don't get validated by HTMLForm::trySubmit() but if something needs to be validated and is invalid, the form that's returned shows every invalid input (even if it doesn't matter). This patch solves that by manually validating based on the prerequisite inputs being checked.

https://gerrit.wikimedia.org/r/665221 Clean up formData before writing it to db
tl;dr - there's no cleanup step so unnecessary values get written into the db even if nothing will be done with them (since the prerequisite "use this eligibility requirement" checkbox wasn't checked). This patch checks for those children props and unsets them so they don't get written in and then deletes them if they exist so that stale values don't linger.

But wait, there's more!

This last one is the interesting one since while rummaging around in the write steps, I noticed that the job that automatically generates an eligibility list always expects that list_edits-startdate and list_edits-enddate which incidentally are the mandatory fields that halted all processing even if they weren't in use, spawning this ticket. 💥

static $listProps = [
  'list_edits-startdate',
  'list_edits-enddate',
  'list_exclude-groups',
  'list_include-groups',
];

I did investigate this further to make sure that I hadn't caused some problem deep in the stack and...I don't think so? In a very technical sense. Basically, we stub out a $params array and then fill it in with data we retrieve from the db and the $listProps loop is part of that. That $params array gets serialized and used to create a job key based on the SHA1 hash. So if something changes, then the hash changes, and the job is new and unique. Since those fields weren't in use (because the eligibility is not in use), this shouldn't negatively impact the purpose of this function.

What I couldn't figure out is why this was a part of that $listProps check* in the first place, given that 1. the requirement might not be in use and 2. there's an explicit conditional to check if it's in use later in the function.
*since there is no isset check when looping through, everything in $listProps must exist as an index.

Also, maybe more relevant, this feature that required the field that caused the initial symptom...doesn't work. I checked back to before we started working with it (pre-OOUI conversion) and even then, I couldn't get a list to generate. I talked to @jrbs who confirmed that...he also doesn't know how this feature works and that it just hasn't been used in a very very long time.

This is the fieldset I'm talking about:

And when I try to "automatically populate" a list, I get

which, fun fact, can be manually edited and as such seems to defeat the point of an auto-generated list to me, considering there are manual overrides elsewhere. If I don't add myself to this list (manually), I cannot vote even though I fulfilled the criteria I set in the generation parameters. I also tested this out in the "before us" state and confirmed that it was non-functional when we got here. It could be that a database migration occurred and left this feature behind but I paused my investigation here.

I asked Joe what if he thought he'd use the feature:

nah. In theory it might be good for local elections with weird restrictions but those use external scripts at the moment anyway

Given all of this information, what do we want to do about this feature? Fix*? Remove? Something else? @Niharika @drochford

*When I say "fix" I mean "figure out what it should do and make sure it does that" because it's still not clear to me what the end result should be.

In hindsight, maybe I should have tagged you earlier @Prtksxna but this ticket went all over the place. I suppose the major decisions I made were:

• users should not see errors for inputs they don't expect to use
• in no-js, the user can see all fields. If a user fills out fields that shouldn't be filled out (for instance, if you haven't checked "Must not be blocked on too many attached wikis" you shouldn't be able to fill out "Central block threshold"), the form passes anyway but doesn't save the value since with js you shouldn't be able to see that field at all or fill it in. Without this, you can do a lot of weird things in no-js that don't impact how eligibility works but that make the form really confusing.

Should these be the case? Or should there be another user flow?

@STran, both these decisions make sense to me, thank you.

@STran Wow, thanks for that writeup. SecurePoll never ceases to be fun. For the eligibility list problem, I created another task - T275470: Add option to allow specific user groups to vote to 'Basic options' list to talk about it. Please verify that I captured the issue correctly.
As for this task, can we resolve it without making it a dependency on that one? If not, then we can fast-track work on that one.

Change 665221 merged by jenkins-bot:
[mediawiki/extensions/SecurePoll@master] Clean up formData before writing it to db

https://gerrit.wikimedia.org/r/665221

Change 665210 merged by jenkins-bot:
[mediawiki/extensions/SecurePoll@master] VoterEligibilityPage: Check date validity in a validation callback

https://gerrit.wikimedia.org/r/665210

Change 665211 merged by jenkins-bot:
[mediawiki/extensions/SecurePoll@master] VoterEligibilityPage: Manually validate dynamically required fields

https://gerrit.wikimedia.org/r/665211

As we are going to be removing most or all of the "Eligibility list" feature, I did not test it in much detail.

I mainly checked that a user could still use the "Basic list" on its own without the "Eligibility list" inputs interfering (which was causing this bug). I checked that inputs in the "Basic list" were appropriately added/removed from the database and did some basic validation testing. I tested with and without JS.

Test Environment: vote.wikimedia.beta.wmflabs.org SecurePoll 2.0.0 (67fe30f) 07:56, 5 March 2021.

In T180762#6891022, @Niharika wrote:

@dom_walden We aren't quite decided about the Eligibility list feature. There's some discussion in T275470: Add option to allow specific user groups to vote to 'Basic options' list.

Ah, OK, thanks for pointing that out. Depending on the outcome of that discussion, I may come back to retest this.

I can no longer test this on testwiki as creating the poll now fails as the 'electionadmin' group is missing - did that break from this work, if so how can we get it fixed?

Bug T277353 opened