Generic problem reported upstream: https://secure.phabricator.com/T7661
When a task is edited, the (new/"new") status of the "Security" field is notified regardless of it having been changed:
scfc triaged this task as "Normal" priority. scfc claimed this task. scfc set Security to none.
The first few times I feared I might have accidentally changed the setting.
Changes of that setting should only be notified if it has indeed been changed from the previous value.
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | • mmodell | T87135 Phabricator should only notify changes to the "Security" field if it is indeed changed | |||
| Resolved | • mmodell | T120013 Next Phabricator Upgrade - 2016-02-18 | |||
| Resolved | • mmodell | T122556 Figure out what upstream "Can Edit Task Policies" policy deprecation means for our Spaces/ACL setup | |||
| Resolved | • DStrine | T123317 Report to TPG on new Phabricator Functionality | |||
| Resolved | • csteipp | T125104 Evaluate the feasibility of phasing out the Phabricator Security extension | |||
| Resolved | • ksmith | T127019 Plan announcement of new major phabricator upgrade |
This only happens in tasks imported from bugzilla, the first time someone use the "edit this task".
Another example: T20871#983927
I'm not really sure what could be the cause of this but it's likely an upstream issue. :-/
The last change (by @chasemp) to that line (post T-479), was at https://phabricator.wikimedia.org/rOPUP269b542f567d76359c37d54bc960fb679fd0d1dd but the bug was a problem before that patch (see third example below), so I don't think the issue originates there.
I've drafted this... If it's correct, could someone push it upstream, please? Thanks :)
We have the message "Security changed to none" appearing if someone edits using the "Edit Task" button. Examples:
How can we fix this? Thanks!
(Possibly related: There was an older bug about the message "changed Security from none to none." always appearing during Task Creation, which MModel fixed with https://phabricator.wikimedia.org/rOPUP26192d415b593eb754a73370dd74bfd117d6a412
The current version is at https://phabricator.wikimedia.org/diffusion/OPUP/browse/production/modules/phabricator/data/fixed_settings.yaml )
The Security field is a "custom field" in Phabricator language, and the report should request something like "Custom fields not changing their 'none' value shouldn't report 'changed to none'", or something along these lines.
... and it happens in relation to a custom field and a template that we have created. I really wonder if this is acceptable upstream, unless @mmodell can identify which piece of Phabricator is causing this problem. With my little knowledge of the several pieces at play here, I don't dare submitting this problem upstream.
Actually it happens just in any task, apparently. I created T89473 in Phabricator, updated several times from the actions dialog, but when I edited the task... T89473#1059766
I don't think it is a regression. When T479 was fixed we got this new type of message.
If rOPUP26192d415b59 was about setting default: default instead of default: none when creating a task, can it be that editing a task still sets default: none? Bare with me, I'm speaking blindly without looking at the code.
I guess this can also include other fields such as the "Is Sprint" field (added by the sprint extension) which also gets added to project logs the same way.
I can't reproduce on phab-01 (https://phab-01.wmflabs.org/T721). Are we using a different version of Phabricator over there?
"Is Sprint" is another custom field, so again not a good case for a report upstream.
As far as I'm aware, our Security extension is not enabled in phab-01, only here in production.
I wonder whether rOPUP269b542f567d changed anything here. I have noted that now the message is
... set Security to None.
Now "None" is uppercase. Was it before?
In T479 the message was
... changed Security from none to None.
See the lowercase first, and the uppercase next.
Also, before the message said "changed" while now is "set". Does it mean that tasks in fact don't have a security variable "set" until the description is edited?
'None' wasn't uppercase before and that fixed things for me. I don't see this problem anymore for edited tasks. The set message still shows up for new tasks but I believe that is the way things are supposed to behave.
"Set" is only used when the task is first created (when the field is changed from null to 'None') and "Changed" is used when any other changes are made to the field (including changes from 'none' to 'None').
Oh I see what you mean. That is interesting. It seems the field isn't initialized if the description isn't set. Phabricator must order the fields so that if a field is edited farther down it changes the fields differently.
Is it just me and my subjective (lack of) memory, or we haven't seen this message for a while now?
Yesterday: T98425
It happens the first time a task is edited after its creation (using the full form, not the comment form), on every task I think.
I've tried everything I can think of to fix this, I really don't know of any solution.
This issue is still present. After creating a new issue, I edit the description but see "Niedzielski set Security to None." in my notification mail.
@Niedzielski: it may still be happening but it doesn't seem like there is much hope of fixing it.
@Qgil: there is actually a better solution than spaces - epriestley implemented a "subscribers" policy which does exactly what our existing "software security bug" does - and template tasks can be used to pre-select the appropriate policy and CC list for a task.
It happens even if there are no other changes being made to the task. e.g. @Liuxinyu970226 now on T114978#1806221 . Can that be stopped somehow. e.g. a mass database change to set the security field to None for tasks without any value in the security field.
A similar idea was suggested in T479#840405 and mentioned again in T479#983947.
I'm not sure why this issue is so intractable.
We can close this, after we updated phab. The security extension is obsolete after that, so no need to fix this.