Skip to content

Feature/2589 search engine mvp#3559

Merged
josepjaume merged 22 commits intomasterfrom
feature/2589-search_engine_MVP_alt2
Jun 6, 2018
Merged

Feature/2589 search engine mvp#3559
josepjaume merged 22 commits intomasterfrom
feature/2589-search_engine_MVP_alt2

Conversation

@tramuntanal
Copy link
Copy Markdown
Contributor

@tramuntanal tramuntanal commented Jun 1, 2018

🎩 What? Why?

Implements the search engine MVP as defined in #2589

📌 Related Issues

📋 Subtasks

  • Add CHANGELOG entry
  • Add documentation regarding the feature
  • Add/modify seeds
  • Add tests

@ghost ghost assigned tramuntanal Jun 1, 2018
@ghost ghost added the status: WIP label Jun 1, 2018
@tramuntanal
Copy link
Copy Markdown
Contributor Author

After adding pg_search and modifying Gemfile.locks and core's gemspec build succeeds. Let's add changes from decidim-core and decidim-dev one after the other.

@tramuntanal tramuntanal force-pushed the feature/2589-search_engine_MVP_alt2 branch from 4f3237b to 8ef29eb Compare June 1, 2018 11:42
@tramuntanal
Copy link
Copy Markdown
Contributor Author

After adding Searchable into decidim-core and decidim-dev tests are still passing. Going to add search into meetings.

@josepjaume josepjaume mentioned this pull request Jun 4, 2018
4 tasks
@tramuntanal
Copy link
Copy Markdown
Contributor Author

After adding Searchable into decidim-meetings tests are still passing. Going to add search into proposals.

@tramuntanal tramuntanal force-pushed the feature/2589-search_engine_MVP_alt2 branch from 7da3a1c to d96d362 Compare June 4, 2018 15:40
@tramuntanal
Copy link
Copy Markdown
Contributor Author

Well, I was wrong. There were missing files in the previous push for meetings and now, after adding those files, the problem at decidim-generatos is raising. So we already have constrained the problem to decidim-meetings (probably proposals will have the same problem because both modules implement search the same way, but let's first study the problem in meetings).

I'm going to progressively, try:

  • comment tests in meetings to see if there is some test causing the problem.
  • comment the inclusion of Searchable in meetings.

I guess some of the previous should be the culprit, if not, I'll see.

@tramuntanal
Copy link
Copy Markdown
Contributor Author

Discarding decidim-meetings tests for search as culprits. Let's try to comment the searchable_fields declaration form Metting, but keep the inclusion of Decidim::Searchable.

@tramuntanal
Copy link
Copy Markdown
Contributor Author

Tests were green after modifying Meetings to include Searchable, but failed after adding tests and migrations. I'm suspecting first of the migration.

@mrcasals
Copy link
Copy Markdown
Contributor

mrcasals commented Jun 5, 2018

@tramuntanal in your migrations you need to use a fake class for the models, maybe the error comes from this? 😕

@tramuntanal
Copy link
Copy Markdown
Contributor Author

tramuntanal commented Jun 5, 2018

After refactoring the meetings migration decidim-generators problem has disappeared. Let's keep adding the rest of the code progressively and also refactor the same way the analogue migration in proposals.

@mrcasals
Copy link
Copy Markdown
Contributor

mrcasals commented Jun 5, 2018

Wohooo!!!!

@tramuntanal
Copy link
Copy Markdown
Contributor Author

Uooo yeahhh!!!

@tramuntanal
Copy link
Copy Markdown
Contributor Author

This PR is now ready to be merged!

@tramuntanal
Copy link
Copy Markdown
Contributor Author

@decidim/lot-core This PR is now ready to be merged!

@josepjaume
Copy link
Copy Markdown
Contributor

Hi @tramuntanal - nice to see it's finally ready! For traceability purposes, do you think you can change the title and description of the PR so it matches the original one? Future developers might struggle to figure out what happened here ;)

@tramuntanal
Copy link
Copy Markdown
Contributor Author

I can remove the "reconstruct incrementally" part of the title because the rest of the title describes best what is the PR about: the search engine MVP.

@tramuntanal tramuntanal changed the title Feature/2589 search engine mvp reconstruct incrementally Feature/2589 search engine mvp Jun 5, 2018
@ghost ghost added the status: WIP label Jun 5, 2018
@tramuntanal
Copy link
Copy Markdown
Contributor Author

@decidim/lot-core re-run tests please, a random test failure has appeared.

@ghost ghost added the status: WIP label Jun 6, 2018
@josepjaume josepjaume merged commit 4423137 into master Jun 6, 2018
@josepjaume josepjaume deleted the feature/2589-search_engine_MVP_alt2 branch June 6, 2018 08:37
@josepjaume
Copy link
Copy Markdown
Contributor

🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉

This was referenced Jun 6, 2018
@mrcasals
Copy link
Copy Markdown
Contributor

mrcasals commented Jun 6, 2018

Wohoooooo! 🎉 🎊 Nice job @tramuntanal! 😄

@agustibr
Copy link
Copy Markdown
Contributor

agustibr commented Jun 6, 2018

🎉 👏 👏

@tramuntanal
Copy link
Copy Markdown
Contributor Author

yehaaaaa!!! 👏 👏

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.

4 participants