If a non-admin user without the "Administer projects" permission tries to edit an existing release node, they get a validation error with a blank title and version string field, even though both fields are disabled. This originally came up while trying to fix #376666: Release node created without title by "preview". Might be related to #369889: New release nodes loose version string and project association, especially since there's a different form_alter code path for editing release nodes.
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | project_release_fix_edit.patch | 901 bytes | hunmonk |
| #3 | release_infos.txt | 2.36 KB | fago |
Comments
Comment #1
pwolanin commentedYep - just saw this trying to edit http://drupal.org/node/378670/edit
Comment #2
Freso commentedSubscribing. =)
Comment #3
fagoYep. As I wasn't able to fill in infos at the first place (#376666: Release node created without title by "preview") I need to set some release infos for my release http://drupal.org/node/377084.
Could some one with proper permissions fix it please? I've added a file with the to-be-set text. Thanks!
Comment #4
hunmonk commentedmight this be a good time to reconsider how this functionality operates? as i recall, a long standing request has been for users to be able to edit HEAD release nodes themselves to point them at new named branches (ie, HEAD is now 6.x-2.x instead of 6.x-1.x). i'm not sure if that's overly complicating things at this point, but i'd rather spend time digging around with an eye towards the way we want things to be ideally here.
Comment #5
dww@hunmonk: that feature already exists, and has existed for a long time. See #89699: make release node form smarter when editing HEAD releases. The only thing not yet working in there is making it so that non-admins can edit the evil "HEAD" version release nodes that were created during the deployment of the "new" release system and give them proper version info. Either way, that's out of scope for this issue.
The problem here should be pretty easy if we stay focused on the bug at hand: D6 doesn't send any values for form elements that are disabled. We've got theseas required fields, but since they're disabled, nothing is sent, so we don't know how to populate them during preview, validation, etc. I propose that if you're not a project admin, and you don't have power to edit these things, they should just show up as #type markup and not be form elements at all (required or not). Or, we could keep them disabled form element, but not mark them as required and make sure we always populate them from the $node, not from form_state['values'] etc.
Comment #6
hunmonk commented@dww: thanks for the clarification.
i vote to convert them to a #value, and simple add a display #item type so the non-admin can see them. sound like a reasonable approach?
Comment #7
longwaveSubscribing, as this just bit me when trying to edit http://drupal.org/node/173678
Comment #8
nancydruHere by way of #383560: Cannot edit release notes
Comment #9
hunmonk commentedi'm not sure if i'm committing a cardinal sin of FAPI here or not, but simply declaring #value explicitly any time the fields are disabled seems to fix the problem. values are preserved for non-admins and editable by admins.
i also used the same fix for the version elements, as they weren't getting values stored when disabled, either...
Comment #10
dwwI had chx comment on this via IRC. He said setting #value for disabled elements is perfectly valid (and something core should consider just doing all the time itself). In reviewing/testing this, I realized there's a whole block of code that was trying to work-around this problem which we can safely remove. See http://drupal.org/cvs?commit=175914. Committed both changes to HEAD, and deployed on d.o. Thanks for your help on this, hunmonk!
@fago: Please try to edit the release yourself and let us know how it goes. ;)
Comment #11
nancydruI edited mine successfully. Derek, you're fabulous!
Comment #12
dwwThanks! /me blushes. ;)
Comment #13
Freso commented