Enabling and disabling Search API is useful when running with multiple development and production environments.

Comments

codesidekick’s picture

This patch adds the enabled property.

What would be better is if a list of excluded parameters (module, id and name) were provided and a warning was shown to the user if they tried to override one of those protected variables. All other variables could then override. In the end because this is environment configuration developers should be given flexibility rather than limited to what they can and can't override.

The function that I patched contained a fair bit of romantic commenting "Using 'we' throughout instead of explaining what is being done in an absolute form.

An example would be

// We can do this for the description too.

Is a bit romantic whereas

  // Override description property.

Is more absolute, and although a bit dry arguably more professional (though it comes down to personal taste).

In this patch I didn't use romantic commenting but feel free to change the comment I added if you want it to conform to the rest of the function.

grayle’s picture

I noticed there was a request for the enabled setting as well as one for the class, so I made a patch which allows everything to be overridden.

nicksanta’s picture

Status: Active » Reviewed & tested by the community

Patch from #2 works.

damienmckenna’s picture

Assigned: codesidekick » Unassigned

FYI you should only set the "assigned" value to your name if you're continuing to work on an issue, if you've finished your proposed changes then you should leave it unassigned.

joelpittet’s picture

Status: Reviewed & tested by the community » Needs work

Can this be done without PHP needing to be updated? aka without array_replace_recursive?

joelpittet’s picture

#1, does enough to cover this, though needs a comments like provided in #2. The code in #2 may be overstepping the scope of this issue a bit, though it is nice, it would be better if it didn't add the extra array_replace_recursive, and the keyword fnc is a bit cryptic.

joelpittet’s picture

Status: Needs work » Needs review
StatusFileSize
new984 bytes

This is just the patch from #1 with the documented param.

grayle’s picture

Reroll of #2, haven't been able to test it though. Not sure it's needed anymore either, I think the current dev version + #7 already has everything except machine_name? It's also, well, too much code for what needs to happen. Yes, you can easily add support for another array like options but the chances that search_api is going to add something like that now is slim to none. I just like writing code and I had some time, so it's here if you want to give it a shot.

vuil’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

The patch of #8 needs to be re-rolled.

solideogloria’s picture

Patch #8 uses a remake of array_replace_recursive() with the motivation that D7 supported PHP < 5.3 so it might not exist. This is no longer the case, since D7 only supports PHP 5.3 and up, so I would think that the patch could be made simpler by using array_replace_recursive() now.

solideogloria’s picture

Status: Needs work » Needs review

@vuil I don't think #8 should be used. Those changes to filter the parameters should be a separate issue.

#7 does what the issue "says on the box".

vuil’s picture

OK, Thanks @solideogloria! I will review it.

joelpittet’s picture

Issue tags: -Needs reroll
StatusFileSize
new1.05 KB

Rerolled #7

  • vuil committed 81f3ad0 on 7.x-1.x authored by joelpittet
    Issue #2478349 by joelpittet, Grayle, codesidekick, vuil, DamienMcKenna...
vuil’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

bjaxelsen’s picture

I think there is a problem with the recent commit.

It should be
$component->enabled
rather than
$server->enabled

solideogloria’s picture

@bjaxelsen #3109309: TypeError and Undefined variable due to merge conflicts.

The fix still needs to be committed and it's been a while. Maybe someone could ping a maintainer?

solideogloria’s picture

Also, this issue should probably be reopened until it's fixed, but only maintainers can reopen.

vuil’s picture

Status: Closed (fixed) » Needs work

I set the issue back to Needs work.

solideogloria’s picture

Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.