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

Create SolrHelper module. #15284

Merged
merged 1 commit into from May 23, 2017
Merged

Create SolrHelper module. #15284

merged 1 commit into from May 23, 2017

Conversation

ashercodeorg
Copy link
Contributor

Mostly a copy-and-paste, with a bug fix as well. This PR is motivated by wanting access to the delete_document method from #15210.

end

def update_document_type_user(db_id, solr_id)
db_user = DASHBOARD_DB[:users].find_by_id(db_id)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The bug fix is here, this should be using the where syntax.

Copy link
Contributor

Choose a reason for hiding this comment

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

it's not clear to me why where().first is better than find

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because find is ActiveRecord syntax, not Sinatra syntax. Though I'd consider it less readable as corrected, it works this way. :)

@ashercodeorg
Copy link
Contributor Author

Note that I'm choosing not to put this in lib/cdo/solr.rb. Feel welcome to suggest that this stuff belongs there.

@Bjvanminnen
Copy link
Contributor

I dont have much/any expertise in this area of the code, but change looks fine to me.

end

def update_document_type_user(db_id, solr_id)
db_user = DASHBOARD_DB[:users].find_by_id(db_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

it's not clear to me why where().first is better than find

@ashercodeorg ashercodeorg merged commit f63ae06 into staging May 23, 2017
@ashercodeorg ashercodeorg deleted the solrRefactor branch May 23, 2017 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants