Closed Bug 544528 Opened 14 years ago Closed 14 years ago

Merge Search v3 (bug 532156) to mobile

Categories

(support.mozilla.org :: Mobile, task)

task
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: jsocol, Assigned: jsocol)

References

()

Details

Attachments

(1 file, 2 obsolete files)

      No description provided.
Mobile forum merge bugs -> 1.5.2.
Target Milestone: --- → 1.5.2
Since this is a 200KB patch, let me list the major changes...

* Add search.php, tiki-admin_search.php, templates/search.tpl, templates/tiki-admin_search.tpl.
* Include the new sphinx.conf, localsettings.py-dist, .txt.dist files in scripts/sphinx.
* Remove old indexers from scripts/sphinx.
* Add the advanced search CSS and JS (webroot/js/mozsadvanced.js, webroot/js/mozsadvanced.css) and add them to minify.conf.php.dist.
* Add the new SphinxLib, the suggestions feature, and dictionaries in webroot/lib/search/
* Add the jquery-ui JS and CSS.
* Add webroot/images/search/ (grab them from trunk)
* Modify the templates to point to "{$tikiroot}search.php" instead of "/tiki-newsearch.php"
* Add search styles to mozcommon.css
* Add the redirect to the top of tiki-newsearch.php

That's it. Search v3 in one colossal package. There are no SQL patches, iirc.

Indexing took <1 locally, and that was with a cold disk cache.

We'll obviously need some IT help to get staging and eventually production moved over, but it's really easy to get everything set up, and nothing chizu hasn't done before.
Assignee: nobody → james
Attachment #425556 - Flags: review?(laura)
(In reply to comment #2)
> There are no SQL patches, iirc.
Minor, but once we move to search v3, we can drop the unused SQL tables se_*
Btw, did we ever do that on SUMO?
(In reply to comment #3)
> Minor, but once we move to search v3, we can drop the unused SQL tables se_*
> Btw, did we ever do that on SUMO?

Check sumotools.
Attachment #425556 - Flags: review?(laura) → review?(paulc)
After all the check-ins in the past couple days, there was bound to be a conflict somewhere. New version, after a brand-new svn up.
Attachment #425556 - Attachment is obsolete: true
Attachment #426280 - Flags: review?(paulc)
Attachment #425556 - Flags: review?(paulc)
Attachment #426280 - Flags: review?(paulc) → review-
Comment on attachment 426280 [details] [diff] [review]
new version, after the latest svn up

Just minor things, really. See below. I looked through the code but didn't see anything else. I assume most of it is identical to trunk?

>Index: webroot/templates/search.tpl
>===================================================================
>+                    <option value="{$k|escape}"{if in_array($k, $smarty.get.forumid) || (1 == $k && !$smarty.get.forumid)}  selected ="selected"{/if}>{tr}{$opt}{/tr}</option>
Should probably change "1 ==" to "5 ==", to default to the mobile forum. (line 107)

>
>Index: webroot/templates/styles/mozms2/searchbar.tpl
>===================================================================
There's another tiki-newsearch.php reference on line 38 of this file. See the trunk version -- looking at the diff, you may just want to copy that one. Minor, since we don't use mozms2 on mobile.


>Index: webroot/templates/modules/mod-search_sidebar.tpl
>===================================================================
This change is not on trunk. Should we add it? Pretty sure this isn't used anywhere (was replaced by the db sidebar module), but it can't hurt to minimize the diff between the two codebases.


>Index: scripts/sphinx/search.conf.php.dist
>===================================================================
>+define('SEARCH_DEFAULT_FORUM', 1);
This is the important: basic search returns no forum results with this value.
We should change this to 5 (the mobile support forum), otherwise IT will have to make those changes -- basic search returns no forum results, it uses this value.

>Index: scripts/sphinx/indexer.sh
>Index: scripts/sphinx/indexer-forums.php
>Index: scripts/sphinx/indexer.php
Just a heads up, applying the patch didn't actually remove these files, so you have to actually remove them if you reapply it.
(In reply to comment #7)
> (From update of attachment 426280 [details] [diff] [review])
> Just minor things, really. See below. I looked through the code but didn't see
> anything else. I assume most of it is identical to trunk?

Yeah, the vast majority is either a section or file copied&pasted from trunk.

> >Index: webroot/templates/search.tpl
> >===================================================================
> >+                    <option value="{$k|escape}"{if in_array($k, $smarty.get.forumid) || (1 == $k && !$smarty.get.forumid)}  selected ="selected"{/if}>{tr}{$opt}{/tr}</option>
> Should probably change "1 ==" to "5 ==", to default to the mobile forum. (line
> 107)

Fixed.


> >Index: webroot/templates/styles/mozms2/searchbar.tpl
> >===================================================================
> There's another tiki-newsearch.php reference on line 38 of this file. See the
> trunk version -- looking at the diff, you may just want to copy that one.
> Minor, since we don't use mozms2 on mobile.

Copied from trunk.


> >Index: webroot/templates/modules/mod-search_sidebar.tpl
> >===================================================================
> This change is not on trunk. Should we add it? Pretty sure this isn't used
> anywhere (was replaced by the db sidebar module), but it can't hurt to minimize
> the diff between the two codebases.

Fair enough, reverted.


> >Index: scripts/sphinx/search.conf.php.dist
> >===================================================================
> >+define('SEARCH_DEFAULT_FORUM', 1);
> This is the important: basic search returns no forum results with this value.
> We should change this to 5 (the mobile support forum), otherwise IT will have
> to make those changes -- basic search returns no forum results, it uses this
> value.

Changed, but since IT will need to edit this file anyway, it hardly seems crucial.


> >Index: scripts/sphinx/indexer.sh
> >Index: scripts/sphinx/indexer-forums.php
> >Index: scripts/sphinx/indexer.php
> Just a heads up, applying the patch didn't actually remove these files, so you
> have to actually remove them if you reapply it.

patch doesn't delete files, but svn does.
Attachment #426280 - Attachment is obsolete: true
Attachment #426373 - Flags: review?(paulc)
Comment on attachment 426373 [details] [diff] [review]
addresses comment 7

WFM.
I just noticed that the two advanced search images are missing, see trunk/webroot/images/search/

You can add those and post the patch, no need to review.
Attachment #426373 - Flags: review?(paulc) → review+
r62083.

Images were already under version control. Must have gotten merged over after r51742.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
r62085. Figured out the image thing. I blame subversion.
Depends on: 545543
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: