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

326 provide basic search functionality #459

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

DerProfessor
Copy link
Member

Provides a basic search functionality described in #326

@DerProfessor DerProfessor requested a review from phyrog June 28, 2018 09:40

# Returns a search result to the GraphQL API
class SearchResolver
def call(root, arguments, _context)
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Assignment Branch Condition size for call is too high. [17.26/15]
C: Method has too many lines. [38/10]

end
result = result.flatten
graphQLResult = OpenStruct.new(
entries: createEntries(result),
Copy link
Contributor

Choose a reason for hiding this comment

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

W: Useless assignment to variable - graphQLResult.
C: Use snake_case for variable names.

private
def allOrganizationalUnits(result)
search_result = 0
result.each do |element|
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Use snake_case for method names.

elem = element._data["_index"]
if elem == 'user' || elem == 'organization'
search_result+=1
end
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Avoid comparing a variable with multiple items in a conditional, use Array#include? instead.

search_result
end

private
Copy link
Contributor

Choose a reason for hiding this comment

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

W: Useless private access modifier.


::Index::RepositoryIndex.import(create :repository, name: 'Ada/repository', owner: ada)
::Index::RepositoryIndex.import(create :repository, name: 'Bob/repository', owner: bob)
::Index::RepositoryIndex.import(create :repository, name: 'Adc/repository', owner: adc)
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Line is too long. [92/80]

::Index::RepositoryIndex.import(create :repository, name: 'Ada/repository', owner: ada)
::Index::RepositoryIndex.import(create :repository, name: 'Bob/repository', owner: bob)
::Index::RepositoryIndex.import(create :repository, name: 'Adc/repository', owner: adc)
::Index::RepositoryIndex.import(create :repository, name: 'Bob/AdaRepository', owner: bob)
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Line is too long. [95/80]

end

it 'returns the organizational units' do
organizational_units = search_result['entries'].select do |e|
%w(User Organization).include?(e['entry']['__typename'])
end
expect(organizational_units.length).to eq(5)
expect(organizational_units.length).to eq(expected_count_organizational_units)
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Line is too long. [86/80]

end

it 'returns the organizational units' do
organizational_units = search_result['entries'].select do |e|
%w(User Organization).include?(e['entry']['__typename'])
end
expect(organizational_units.length).to eq(5)
expect(organizational_units.length).to eq(expected_count_organizational_units)
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Line is too long. [86/80]

it 'returns the organizational units' do
organizational_units = search_result['entries'].select do |e|
%w(User Organization).include?(e['entry']['__typename'])
end
expect(organizational_units.length).to eq(5)
expect(organizational_units.length).to eq(expected_count_organizational_units)
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Line is too long. [86/80]

)}).entries
end
result = result.flatten
graphQLResult = OpenStruct.new(
Copy link
Contributor

Choose a reason for hiding this comment

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

W: Useless assignment to variable - graphQLResult.
C: Use snake_case for variable names.


private

def allOrganizationalUnits(result)
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Use snake_case for method names.

search_result = 0
result.each do |element|
elem = element._data['_index']
search_result += 1 if elem == 'user' || elem == 'organization'
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Avoid comparing a variable with multiple items in a conditional, use Array#include? instead.


private

def allRepositories(result)
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Use snake_case for method names.

search_result
end

private
Copy link
Contributor

Choose a reason for hiding this comment

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

W: Useless private access modifier.

::Index::OrganizationIndex.import(create(:organization, display_name: 'Abc_Organization'))

::Index::RepositoryIndex.import(create(:repository, name: 'Ada/repository', owner: ada))
::Index::RepositoryIndex.import(create(:repository, name: 'Bob/repository', owner: bob))
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Line is too long. [92/80]


::Index::RepositoryIndex.import(create(:repository, name: 'Ada/repository', owner: ada))
::Index::RepositoryIndex.import(create(:repository, name: 'Bob/repository', owner: bob))
::Index::RepositoryIndex.import(create(:repository, name: 'Adc/repository', owner: adc))
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Line is too long. [92/80]

::Index::RepositoryIndex.import(create(:repository, name: 'Ada/repository', owner: ada))
::Index::RepositoryIndex.import(create(:repository, name: 'Bob/repository', owner: bob))
::Index::RepositoryIndex.import(create(:repository, name: 'Adc/repository', owner: adc))
::Index::RepositoryIndex.import(create(:repository, name: 'Bob/AdaRepository', owner: bob))
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Line is too long. [95/80]

end

it 'returns the organizational units' do
organizational_units = search_result['entries'].select do |e|
%w(User Organization).include?(e['entry']['__typename'])
end
expect(organizational_units.length).to eq(5)
expect(organizational_units.length).to eq(expected_count_organizational_units)
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Line is too long. [86/80]

it 'returns the organizational units' do
organizational_units = search_result['entries'].select do |e|
%w(User Organization).include?(e['entry']['__typename'])
end
expect(organizational_units.length).to eq(5)
expect(organizational_units.length).to eq(expected_count_organizational_units)
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Line is too long. [86/80]

@codecov-io
Copy link

codecov-io commented Jun 28, 2018

Codecov Report

Merging #459 into master will decrease coverage by <.01%.
The diff coverage is 98.7%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #459      +/-   ##
==========================================
- Coverage     100%   99.99%   -0.01%     
==========================================
  Files         399      399              
  Lines       10675    10696      +21     
==========================================
+ Hits        10675    10695      +20     
- Misses          0        1       +1
Impacted Files Coverage Δ
spec/graphql/queries/search_query_spec.rb 100% <100%> (ø) ⬆️
app/graphql/types/query_type.rb 100% <100%> (ø) ⬆️
app/graphql/types/search_result_type.rb 100% <100%> (ø) ⬆️
lib/search_resolver.rb 96.96% <96.96%> (ø)
app/models/search_result.rb

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 96bf8b5...547dc78. Read the comment docs.

private
def allOrganizationalUnits(result)
search_result = 0
result.each do |element|
Copy link
Contributor

Choose a reason for hiding this comment

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

Check out Enumerable#count

private
def allRepositories(result)
search_result = 0
result.each do |element|
Copy link
Contributor

Choose a reason for hiding this comment

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

Check out Enumerable#count

::Index::OrganizationIndex::Organization,
::Index::UserIndex::User]
else
indices = categories.map do |category|
Copy link
Contributor

@phyrog phyrog Jun 28, 2018

Choose a reason for hiding this comment

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

Check out Enumerable#reduce:

indices = categories.reduce([]) do |indices, category|
  ...
    when 'repositories'
      indices + [::Index::RepositoryIndex::Repository]
  ...
end

that way you don't need to flatten the array at the end, which might make it faster.

end
indices = indices.flatten
end
result = indices.map do |index|
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be done using reduce as well, but I think here flatten is fine.

)
end

def search_index(query, indices)
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Method has too many lines. [11/10]

end

def create_indices(categories)
if categories.blank?
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Method has too many lines. [17/10]

result.each do |element|
elem = element._data['_index']
if elem == 'user' || elem == 'organization'
search_result += 1
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Avoid comparing a variable with multiple items in a conditional, use Array#include? instead.

search_result += 1
end
end
search_result
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Assignment Branch Condition size for create_entries is too high. [19.1/15]
C: Method has too many lines. [13/10]

result.flatten
end

def create_indices(categories)
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Method has too many lines. [17/10]

search_result = 0
result.each do |element|
elem = element._data['_index']
search_result += 1 if elem == 'user' || elem == 'organization'
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Avoid comparing a variable with multiple items in a conditional, use Array#include? instead.

search_result
end

def create_entries(result)
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Assignment Branch Condition size for create_entries is too high. [19.1/15]
C: Method has too many lines. [13/10]

require 'ostruct'

# Returns a search result to the GraphQL API
class SearchResolver
Copy link
Contributor

Choose a reason for hiding this comment

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

E: class definition in method body (Using Ruby 2.5 parser; configure using TargetRubyVersion parameter, under AllCops)

def all_organizational_units(result)
result.count do |element|
elem = element._data['_index']
<<<<<<< HEAD
Copy link
Contributor

Choose a reason for hiding this comment

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

E: unexpected token tLSHFT (Using Ruby 2.5 parser; configure using TargetRubyVersion parameter, under AllCops)

elem = element._data['_index']
<<<<<<< HEAD
search_result += 1 if elem == 'user' || elem == 'organization'
=======
Copy link
Contributor

Choose a reason for hiding this comment

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

E: unexpected token tEQQ (Using Ruby 2.5 parser; configure using TargetRubyVersion parameter, under AllCops)

search_result += 1 if elem == 'user' || elem == 'organization'
=======
elem == 'organization' || elem == 'user'
>>>>>>> Obey review comments
Copy link
Contributor

Choose a reason for hiding this comment

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

E: unexpected token tRSHFT (Using Ruby 2.5 parser; configure using TargetRubyVersion parameter, under AllCops)

end

def all_repositories(result)
<<<<<<< HEAD
Copy link
Contributor

Choose a reason for hiding this comment

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

E: unexpected token tLSHFT (Using Ruby 2.5 parser; configure using TargetRubyVersion parameter, under AllCops)

result.each do |element|
elem = element._data['_index']
search_result += 1 if elem == 'repository'
=======
Copy link
Contributor

Choose a reason for hiding this comment

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

E: unexpected token tEQQ (Using Ruby 2.5 parser; configure using TargetRubyVersion parameter, under AllCops)

=======
result.count do |element|
element._data['_index'] == 'repository'
>>>>>>> Obey review comments
Copy link
Contributor

Choose a reason for hiding this comment

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

E: unexpected token tRSHFT (Using Ruby 2.5 parser; configure using TargetRubyVersion parameter, under AllCops)

element._data['_index'] == 'repository'
>>>>>>> Obey review comments
end
end
Copy link
Contributor

Choose a reason for hiding this comment

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

E: unexpected token kEND (Using Ruby 2.5 parser; configure using TargetRubyVersion parameter, under AllCops)

)
end
end
end
Copy link
Contributor

Choose a reason for hiding this comment

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

E: unexpected token kEND (Using Ruby 2.5 parser; configure using TargetRubyVersion parameter, under AllCops)

::Index::OrganizationIndex::Organization,
::Index::UserIndex::User]
else
indices = categories.reduce([]) do |indices, category|
Copy link
Contributor

Choose a reason for hiding this comment

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

W: Useless assignment to variable - indices.
W: Shadowing outer local variable - indices.

end
end

def create_entries(result)
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Assignment Branch Condition size for create_entries is too high. [19.1/15]
C: Method has too many lines. [13/10]

end
end

def reduce_categories(categories)
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Method has too many lines. [11/10]

end.flatten
end

def create_indices(categories)
Copy link
Contributor

Choose a reason for hiding this comment

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

map_categories_to_indices might be a better name

end
end

def all_organizational_units(result)
Copy link
Contributor

Choose a reason for hiding this comment

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

organizational_units_count might be a better name

end
end

def all_repositories(result)
Copy link
Contributor

Choose a reason for hiding this comment

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

repositories_count might be a better name

end
end

def create_entries(result)
Copy link
Contributor

Choose a reason for hiding this comment

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

map_entries_to_models might be a better name

when 'repositories'
indices + [::Index::RepositoryIndex::Repository]
else
indices + []
Copy link
Contributor

Choose a reason for hiding this comment

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

can be replaced by just indices as concatenating an empty list is a no-op.

end
end

def map_entries_to_models(result)
Copy link
Contributor

Choose a reason for hiding this comment

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

C: Assignment Branch Condition size for map_entries_to_models is too high. [19.1/15]
C: Method has too many lines. [13/10]

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.

4 participants