Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Section pages search #1038

Merged
merged 14 commits into from
Oct 24, 2017
Merged

Section pages search #1038

merged 14 commits into from
Oct 24, 2017

Conversation

apradillap
Copy link
Contributor

@apradillap apradillap commented Oct 23, 2017

Connects to #1014

What does this PR do?

To look for in the sections within the pages. Front autocomplete has been reused.

How should this be manually tested?

Go to /admin/cms/sections, create a section and it looks for in the pages

Reindex

Add collection to the indexing of the pages, so reindex it.
bundle exec rake algoliasearch:reindex

@apradillap apradillap changed the title 1014 section pages search Section pages search Oct 23, 2017
@apradillap apradillap self-assigned this Oct 23, 2017
@codecov-io
Copy link

codecov-io commented Oct 23, 2017

Codecov Report

Merging #1038 into master will decrease coverage by 0.23%.
The diff coverage is 10%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1038      +/-   ##
=========================================
- Coverage   78.03%   77.8%   -0.24%     
=========================================
  Files         421     421              
  Lines       10869   10888      +19     
=========================================
- Hits         8482    8471      -11     
- Misses       2387    2417      +30
Impacted Files Coverage Δ
...gobierto_admin/gobierto_cms/sections_controller.rb 0% <0%> (ø) ⬆️
app/models/gobierto_cms/page.rb 94% <100%> (ø) ⬆️
...s/gobierto_participation/issues/base_controller.rb 0% <0%> (-100%) ⬇️
app/controllers/gobierto_admin/base_controller.rb 89.74% <0%> (-2.57%) ⬇️
app/models/gobierto_budgets/budget_line_stats.rb 85.91% <0%> (+1.4%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55d68dd...0a0e205. Read the comment docs.

Copy link
Member

@ferblape ferblape left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a couple of details to fix!

} else {
$('<div class="result"><p>No hay resultados</p></div>').appendTo($resultsContainer);
$('<div class="result"><p>'+I18n.t("layouts.search.no_results")+'</p></div>').appendTo($resultsContainer);
}

$('<div class="result"><small>'+I18n.t("layouts.search.powered_by")+'</small></div>').appendTo($resultsContainer);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a small detail, but that message is not very useful in the admin UI. Could we add a flag to remove the powered by if we are in the admin?

if(window.searchClient.indexes.length == 1) {
var result = '<div class="activity_item">' +
'<h2>' + '<a class="tipsit" href=' + ["/admin/cms/pages/", d['objectID'], "/edit?collection_id=", d['collection_id']].join('') +
' original-title="Arrastra y suelta en la parte izquierda para colocar esta página en el menú">' +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing translations

<script type="text/javascript">
window.searchClient = {
client: algoliasearch('<%= Rails.application.secrets.algolia_application_id %>', '<%= Rails.application.secrets.algolia_search_api_key %>'),
indexes: ["gobierto_populate_<%= Rails.env %>_GobiertoCms::Page"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use this method to get the index name.

@apradillap apradillap removed the wip label Oct 24, 2017
@apradillap apradillap merged commit c561694 into master Oct 24, 2017
@apradillap apradillap deleted the 1014-section-pages-search branch October 24, 2017 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants