Skip to content

Add pending requirements#2

Merged
grillermo merged 39 commits intomasterfrom
add-pending-requirements
Jan 12, 2026
Merged

Add pending requirements#2
grillermo merged 39 commits intomasterfrom
add-pending-requirements

Conversation

@grillermo
Copy link
Copy Markdown
Owner

@grillermo grillermo commented Dec 17, 2021

This PR includes all the code of the original PR and the missing requirement:

  • The results should show the path of introduction from Alan to the expert e.g. Alan wants to get introduced to someone who writes about 'Dog breeding'. Claudia's website has a heading tag "Dog breeding in Ukraine". Bart knows Alan and Claudia. An example search result would be Alan -> Bart -> Claudia ("Dog breeding in Ukraine").

Technical decisions

I solved it building a Graph each time a search for potential friends is performed, we could easily cache that graph and just update its connections each time a friendship is made. But for now it should be a good example of the bread first search algorithm used to find the shortest path between two nodes.

    I also converted the application from an API only to a regular rails app, assuming that a backend challenge would also be ok without the heavy frontend work needed to use an API.
    The command was
    rails new backend-challenge
    That overrides the existing files and adds the missing ones to have a regular frontend + backend rails app.
nicer alternative to erb
Substitute default rails welcome page
We are already testing the behavior on the members controller spec but still...
Devise already tests the authenticate_member! method we are using so we don't need to test it again
It will handle the authentication

The code is the result of running
rails generate devise:install
rails generate devise Member
Our .rspec file already requires the spec_helper on every spec file so move all the test config there to avoid having to require it on every file individually
It provides a classless styles, so good defaults with minimal changes to the markup
I went with sucker punch and not Sidekiq to avoid adding the broker dependency(again, Redis) and to allow easy testing for the reviewer
It will be used to store the member headings
We will be accessing urls so we need a way to record the results to have a completely offline test suite
This job will pull the headings for a member to be called after the member creation
We need to override the Devise controller to be able to access the newly created member
Part of the requirements, i used my own personal url shortner chiq.me
This class will help us explore the connections between members to find connecting paths between them.
This class will return those friend of a friend with maybe shared interests that a member can connect with.
We will need other column to display the potential friends search
The fuzzystrmatch module provides several functions to determine similarities and distance between strings, helpful for our search feature.
Generally application.css only imports others
@grillermo grillermo mentioned this pull request Dec 19, 2021
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

mrge found 70 issues across 80 files. View them in mrge.io

# so we can query it about those relationships and build connections
class MembersSocialGraph
def initialize
@members = Member.all
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fetching all members in initialize may become a performance bottleneck on large datasets since the graph is rebuilt on every search.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

thank you!

end
end

context 'some members have headings that are a substring o the query' do
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Typo in context description: 'a substring o the query' should be 'a substring of the query'

include_examples 'successful search'
end

context 'some members have headings that are sound very to the query' do
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Grammatical error in context description: 'are sound very to the query' is incorrect

returned_member = subject.first

expect(returned_member.path_to_other_member).to be_present
expect(returned_member.path_to_other_member).to eql [
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Test doesn't verify the actual path-finding functionality, only that a path exists


shared_examples 'successful search' do
it 'returns the friend of a friend interested in dog breeding' do
expect(subject).to eql [friend_of_friend]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All three matching strategies use identical test assertions without verifying specific matching behavior

let(:personal_website_url) { 'https://www.joshwcomeau.com/' }
let(:member) { create(:member, personal_website_url: personal_website_url) }
let(:vcr_mode) { :new_episodes } # :all, :none, :new_episodes
let(:vcr_cassette) { :successfull_url_shortner_job }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Rule violated: Avoid Logging Sensitive Information

  VCR cassette will record sensitive API credentials

uri: http://chiq.me/
body:
encoding: US-ASCII
string: password=ik3%7Dcj6WoQLxKvFbMNnnUu7bq&url=https%3A%2F%2Fwww.joshwcomeau.com%2F
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Rule violated: Avoid Logging Sensitive Information

  Password exposed in VCR cassette

# Fetch Request
res = http.request(req)
res
rescue StandardError => e
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Rule violated: Avoid Logging Sensitive Information

  Error logging might expose sensitive information including API credentials

name: 'Josh Comeau',
email: 'hello@joshwcomeau.com',
personal_website_url: 'https://www.joshwcomeau.com/',
password: '123456',
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Rule violated: Avoid Logging Sensitive Information

  Test contains hardcoded password credentials

last_name: 'Metz',
url: 'http://www.example.com'
name: 'Josh Comeau',
email: 'hello@joshwcomeau.com',
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Rule violated: Avoid Logging Sensitive Information

  Test exposes real email address which may be logged

@grillermo grillermo merged commit a52a2e9 into master Jan 12, 2026
1 check passed
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.

1 participant