While testing #1044950: Re-architect the project-dropdown UI selector on node/add/project-issue to prevent server crashes I noticed that although the project selection widget at node/add/project-issue itself now honors the 'project_selection_widget' setting, the 'field_project' entityreference field on issue nodes does not (for obvious reasons). Instead, if you want to change that, you have to go into the field configuration and fix it there.

I see a few options here:

A) Whenever we're saving the 'project_selection_widget' setting, we should see if the field configuration matches, and if not, fix it. This is basically impossible, since there's no hook invoked when a variable is set (e.g. via drush) and the setting could be defined in settings.php. So, best we could hope for here is if the setting is changed directly via form submission in the UI, which obviously sucks and isn't worth the trouble.

B) Whenever we save the setting, we at least print a warning about checking the field configuration, too. Most of the same problems from A still apply.

C) ??

Frankly, I don't know how to deal with this. This might need to be set to "won't fix" but I at least wanted to open an issue to discuss the desirability and feasibility of fixing this...

-Derek

Comments

bdragon’s picture

bdragon commented

C) Do some magic with features and alter hooks?

dww’s picture

dww
we/he/they
commented
Issue tags: +4hr

For the purposes of the d.o launch, all we're going to do is go through all of our node type + field configuration to make sure this is done properly. Once that's done, we can untag this for the Drupal.org D7 and punt this down the road (perhaps to eventually be won't fix'ed, anyway).

drumm’s picture

drumm
he/him
Location NY, US
commented
Assigned: Unassigned » drumm
drumm’s picture

drumm
he/him
Location NY, US
commented
Assigned: drumm » Unassigned
Issue tags: -project, -drupal.org D7, -4hr

I checked all fields and found that field_release_project was hidden, but the bare node/add page had a project selection. http://drupalcode.org/project/project.git/commitdiff/f834cb6670a936ec748... makes it autocomplete when set.

Unassigning and untagging for any future magic.

dww’s picture

dww
we/he/they
commented

Cool, thanks. Mostly looks great. 1 thing makes me nervous:

+ $items['project/autocomplete/release/project/%'] = array(

I'm having flashbacks to #180316: fix project/mymodule URI insanity... ;)

Ahh, cool, upon closer inspection, we already have 'autocomplete' on the blacklist of project machine names (I'm not even sure if that stuff has been ported to D7 and works or not) since project_issue uses a bunch of /project/autocomplete/issue/* callbacks..

drumm’s picture

drumm
he/him
Location NY, US
commented

I think that stuff has been ported to D7 in project_issue. I based my patch on it. The main difference is that project_issue's autocomplete is multiple comma separated projects, and release simplifies it to be only one.

dww’s picture

dww
we/he/they
commented

Sorry, I meant enforcing that blacklist of project machine names.

drumm’s picture

drumm
he/him
Location NY, US
commented

Yep, that works. I just tested and got "This project short name is reserved."

dww’s picture

dww
we/he/they
commented

Great. Yeah, that's from project_validate_project_shortname() which is injected into the node form via project_form_node_form_alter(). It's sort of a hack, it's hard-coding 'field_project_machine_name' whereas it'd be cleaner if it was somehow tied directly to the machine_name field itself. But, whatever, it works for now (and all this is out of scope for this issue, anyway).

Thanks,
-Derek

joachim’s picture

joachim commented

How about:

D. Go the other way round. Ditch the variable, and where we used to read the variable, read the field setting instead.

dww’s picture

dww
we/he/they
commented

Re: #10 and option D: because there are potentially multiple node types configured to behave like issues, so there's no "read the field setting" we can use in more site-wide places where we care. :/

But thanks for the suggestion!

Cheers,
-Derek

joachim’s picture

joachim commented

The places I can find it's used are:

function project_release_pick_project_form($form, &$form_state, $node_type) {
  $form_state['node_type'] = $node_type;

  if (variable_get('project_selection_widget', 'select') === 'select') {

There is a node type in context. So you can read it off the field instance for that node type.

function project_issue_pick_project_form($form, &$form_state, $node_type) {
  $form = array();

  drupal_set_title(t('Submit @name', array('@name' => node_type_get_name($node_type))));

  $form_state['node_type'] = $node_type;

  if (variable_get('project_selection_widget', 'select') == 'select') {

Ditto.

Have I missed any?

dww’s picture

dww
we/he/they
commented

Nearly positive there were more in D6, but in D7, grep agrees with you. ;) So yeah, that should probably work. It's potentially going to make issues like #1822482: Deal with 'create an issue' links for sites with more than 1 issue node type harder, but that's a mess already...

I can't think of any reasons not to do proposal D, although definitely a lower prio than a lot of other issues that are open now.

Thanks!
-Derek