diff --git a/Gemfile b/Gemfile index 5a8ffc43..81244b48 100644 --- a/Gemfile +++ b/Gemfile @@ -19,4 +19,11 @@ gem 'spring', group: :development gem 'turbolinks' gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby] gem 'uglifier' +gem 'concurrent-ruby' +gem 'kaminari' gem 'web-console', group: :development +gem 'factory_bot_rails', '~> 6.2', group: [:development, :test] +gem 'shoulda-matchers', '~> 5.3', group: [:development, :test] +gem 'factory_bot', '~> 6.3', group: [:development, :test] +gem 'rails-controller-testing', group: [:test] +gem 'faker' \ No newline at end of file diff --git a/Gemfile.lock b/Gemfile.lock index 14ec6457..81df0cab 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,87 +1,87 @@ GEM remote: https://rubygems.org/ specs: - actioncable (7.0.4) - actionpack (= 7.0.4) - activesupport (= 7.0.4) + actioncable (7.0.8.4) + actionpack (= 7.0.8.4) + activesupport (= 7.0.8.4) nio4r (~> 2.0) websocket-driver (>= 0.6.1) - actionmailbox (7.0.4) - actionpack (= 7.0.4) - activejob (= 7.0.4) - activerecord (= 7.0.4) - activestorage (= 7.0.4) - activesupport (= 7.0.4) + actionmailbox (7.0.8.4) + actionpack (= 7.0.8.4) + activejob (= 7.0.8.4) + activerecord (= 7.0.8.4) + activestorage (= 7.0.8.4) + activesupport (= 7.0.8.4) mail (>= 2.7.1) net-imap net-pop net-smtp - actionmailer (7.0.4) - actionpack (= 7.0.4) - actionview (= 7.0.4) - activejob (= 7.0.4) - activesupport (= 7.0.4) + actionmailer (7.0.8.4) + actionpack (= 7.0.8.4) + actionview (= 7.0.8.4) + activejob (= 7.0.8.4) + activesupport (= 7.0.8.4) mail (~> 2.5, >= 2.5.4) net-imap net-pop net-smtp rails-dom-testing (~> 2.0) - actionpack (7.0.4) - actionview (= 7.0.4) - activesupport (= 7.0.4) - rack (~> 2.0, >= 2.2.0) + actionpack (7.0.8.4) + actionview (= 7.0.8.4) + activesupport (= 7.0.8.4) + rack (~> 2.0, >= 2.2.4) rack-test (>= 0.6.3) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.2.0) - actiontext (7.0.4) - actionpack (= 7.0.4) - activerecord (= 7.0.4) - activestorage (= 7.0.4) - activesupport (= 7.0.4) + actiontext (7.0.8.4) + actionpack (= 7.0.8.4) + activerecord (= 7.0.8.4) + activestorage (= 7.0.8.4) + activesupport (= 7.0.8.4) globalid (>= 0.6.0) nokogiri (>= 1.8.5) - actionview (7.0.4) - activesupport (= 7.0.4) + actionview (7.0.8.4) + activesupport (= 7.0.8.4) builder (~> 3.1) erubi (~> 1.4) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.1, >= 1.2.0) - activejob (7.0.4) - activesupport (= 7.0.4) + activejob (7.0.8.4) + activesupport (= 7.0.8.4) globalid (>= 0.3.6) - activemodel (7.0.4) - activesupport (= 7.0.4) - activerecord (7.0.4) - activemodel (= 7.0.4) - activesupport (= 7.0.4) - activestorage (7.0.4) - actionpack (= 7.0.4) - activejob (= 7.0.4) - activerecord (= 7.0.4) - activesupport (= 7.0.4) + activemodel (7.0.8.4) + activesupport (= 7.0.8.4) + activerecord (7.0.8.4) + activemodel (= 7.0.8.4) + activesupport (= 7.0.8.4) + activestorage (7.0.8.4) + actionpack (= 7.0.8.4) + activejob (= 7.0.8.4) + activerecord (= 7.0.8.4) + activesupport (= 7.0.8.4) marcel (~> 1.0) mini_mime (>= 1.1.0) - activesupport (7.0.4) + activesupport (7.0.8.4) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 1.6, < 2) minitest (>= 5.1) tzinfo (~> 2.0) - addressable (2.8.1) - public_suffix (>= 2.0.2, < 6.0) - bcrypt (3.1.18) + addressable (2.8.7) + public_suffix (>= 2.0.2, < 7.0) + base64 (0.2.0) + bcrypt (3.1.20) bindex (0.8.1) - builder (3.2.4) + builder (3.3.0) byebug (11.1.3) - capybara (3.37.1) + capybara (3.40.0) addressable matrix mini_mime (>= 0.1.3) - nokogiri (~> 1.8) + nokogiri (~> 1.11) rack (>= 1.6.0) rack-test (>= 0.6.3) regexp_parser (>= 1.5, < 3.0) xpath (~> 3.2) - childprocess (4.1.0) coderay (1.1.3) coffee-rails (5.0.0) coffee-script (>= 2.2.0) @@ -90,124 +90,150 @@ GEM coffee-script-source execjs coffee-script-source (1.12.2) - concurrent-ruby (1.1.10) + concurrent-ruby (1.3.4) crass (1.0.6) - devise (4.8.1) + date (3.3.4) + devise (4.9.4) bcrypt (~> 3.0) orm_adapter (~> 0.1) railties (>= 4.1.0) responders warden (~> 1.2.3) - diff-lcs (1.5.0) - digest (3.1.0) - erubi (1.11.0) - execjs (2.8.1) - ffi (1.15.5) - globalid (1.0.0) - activesupport (>= 5.0) - i18n (1.12.0) + diff-lcs (1.5.1) + erubi (1.13.0) + execjs (2.9.1) + factory_bot (6.4.6) + activesupport (>= 5.0.0) + factory_bot_rails (6.4.3) + factory_bot (~> 6.4) + railties (>= 5.0.0) + faker (3.4.2) + i18n (>= 1.8.11, < 2) + ffi (1.17.0-arm64-darwin) + ffi (1.17.0-x86_64-darwin) + globalid (1.2.1) + activesupport (>= 6.1) + i18n (1.14.5) concurrent-ruby (~> 1.0) - jbuilder (2.11.5) + jbuilder (2.12.0) actionview (>= 5.0.0) activesupport (>= 5.0.0) - listen (3.7.1) + kaminari (1.2.2) + activesupport (>= 4.1.0) + kaminari-actionview (= 1.2.2) + kaminari-activerecord (= 1.2.2) + kaminari-core (= 1.2.2) + kaminari-actionview (1.2.2) + actionview + kaminari-core (= 1.2.2) + kaminari-activerecord (1.2.2) + activerecord + kaminari-core (= 1.2.2) + kaminari-core (1.2.2) + listen (3.9.0) rb-fsevent (~> 0.10, >= 0.10.3) rb-inotify (~> 0.9, >= 0.9.10) - loofah (2.19.0) + logger (1.6.0) + loofah (2.22.0) crass (~> 1.0.2) - nokogiri (>= 1.5.9) - mail (2.7.1) + nokogiri (>= 1.12.0) + mail (2.8.1) mini_mime (>= 0.1.1) - marcel (1.0.2) + net-imap + net-pop + net-smtp + marcel (1.0.4) matrix (0.4.2) - method_source (1.0.0) - mini_mime (1.1.2) - mini_portile2 (2.8.0) - minitest (5.16.3) - net-imap (0.2.3) - digest + method_source (1.1.0) + mini_mime (1.1.5) + minitest (5.24.1) + net-imap (0.4.14) + date net-protocol - strscan - net-pop (0.1.1) - digest + net-pop (0.1.2) net-protocol + net-protocol (0.2.2) timeout - net-protocol (0.1.3) - timeout - net-smtp (0.3.1) - digest + net-smtp (0.5.0) net-protocol - timeout - nio4r (2.5.8) - nokogiri (1.13.8) - mini_portile2 (~> 2.8.0) + nio4r (2.7.3) + nokogiri (1.16.7-arm64-darwin) + racc (~> 1.4) + nokogiri (1.16.7-x86_64-darwin) racc (~> 1.4) orm_adapter (0.5.0) - pg (1.4.3) - pry (0.14.1) + pg (1.5.7) + pry (0.14.2) coderay (~> 1.1) method_source (~> 1.0) - pry-rails (0.3.9) - pry (>= 0.10.4) - public_suffix (5.0.0) - puma (5.6.5) + pry-rails (0.3.11) + pry (>= 0.13.0) + public_suffix (6.0.1) + puma (6.4.2) nio4r (~> 2.0) - racc (1.6.0) - rack (2.2.4) - rack-test (2.0.2) + racc (1.8.1) + rack (2.2.9) + rack-test (2.1.0) rack (>= 1.3) - rails (7.0.4) - actioncable (= 7.0.4) - actionmailbox (= 7.0.4) - actionmailer (= 7.0.4) - actionpack (= 7.0.4) - actiontext (= 7.0.4) - actionview (= 7.0.4) - activejob (= 7.0.4) - activemodel (= 7.0.4) - activerecord (= 7.0.4) - activestorage (= 7.0.4) - activesupport (= 7.0.4) + rails (7.0.8.4) + actioncable (= 7.0.8.4) + actionmailbox (= 7.0.8.4) + actionmailer (= 7.0.8.4) + actionpack (= 7.0.8.4) + actiontext (= 7.0.8.4) + actionview (= 7.0.8.4) + activejob (= 7.0.8.4) + activemodel (= 7.0.8.4) + activerecord (= 7.0.8.4) + activestorage (= 7.0.8.4) + activesupport (= 7.0.8.4) bundler (>= 1.15.0) - railties (= 7.0.4) - rails-dom-testing (2.0.3) - activesupport (>= 4.2.0) + railties (= 7.0.8.4) + rails-controller-testing (1.0.5) + actionpack (>= 5.0.1.rc1) + actionview (>= 5.0.1.rc1) + activesupport (>= 5.0.1.rc1) + rails-dom-testing (2.2.0) + activesupport (>= 5.0.0) + minitest nokogiri (>= 1.6) - rails-html-sanitizer (1.4.3) - loofah (~> 2.3) - railties (7.0.4) - actionpack (= 7.0.4) - activesupport (= 7.0.4) + rails-html-sanitizer (1.6.0) + loofah (~> 2.21) + nokogiri (~> 1.14) + railties (7.0.8.4) + actionpack (= 7.0.8.4) + activesupport (= 7.0.8.4) method_source rake (>= 12.2) thor (~> 1.0) zeitwerk (~> 2.5) - rake (13.0.6) + rake (13.2.1) rb-fsevent (0.11.2) - rb-inotify (0.10.1) + rb-inotify (0.11.1) ffi (~> 1.0) - regexp_parser (2.5.0) - responders (3.0.1) - actionpack (>= 5.0) - railties (>= 5.0) - rexml (3.2.5) - rspec-core (3.11.0) - rspec-support (~> 3.11.0) - rspec-expectations (3.11.1) - diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.11.0) - rspec-mocks (3.11.1) - diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.11.0) - rspec-rails (5.1.2) + regexp_parser (2.9.2) + responders (3.1.1) actionpack (>= 5.2) - activesupport (>= 5.2) railties (>= 5.2) - rspec-core (~> 3.10) - rspec-expectations (~> 3.10) - rspec-mocks (~> 3.10) - rspec-support (~> 3.10) - rspec-support (3.11.1) + rexml (3.3.5) + strscan + rspec-core (3.13.0) + rspec-support (~> 3.13.0) + rspec-expectations (3.13.1) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.13.0) + rspec-mocks (3.13.1) + diff-lcs (>= 1.2.0, < 2.0) + rspec-support (~> 3.13.0) + rspec-rails (6.1.3) + actionpack (>= 6.1) + activesupport (>= 6.1) + railties (>= 6.1) + rspec-core (~> 3.13) + rspec-expectations (~> 3.13) + rspec-mocks (~> 3.13) + rspec-support (~> 3.13) + rspec-support (3.13.1) rubyzip (2.3.2) sass-rails (6.0.0) sassc-rails (~> 2.1, >= 2.1.1) @@ -219,62 +245,73 @@ GEM sprockets (> 3.0) sprockets-rails tilt - selenium-webdriver (4.4.0) - childprocess (>= 0.5, < 5.0) + selenium-webdriver (4.23.0) + base64 (~> 0.2) + logger (~> 1.4) rexml (~> 3.2, >= 3.2.5) rubyzip (>= 1.2.2, < 3.0) websocket (~> 1.0) - spring (4.1.0) - sprockets (4.1.1) + shoulda-matchers (5.3.0) + activesupport (>= 5.2.0) + spring (4.2.1) + sprockets (4.2.1) concurrent-ruby (~> 1.0) - rack (> 1, < 3) - sprockets-rails (3.4.2) - actionpack (>= 5.2) - activesupport (>= 5.2) + rack (>= 2.2.4, < 4) + sprockets-rails (3.5.2) + actionpack (>= 6.1) + activesupport (>= 6.1) sprockets (>= 3.0.0) - strscan (3.0.4) - thor (1.2.1) - tilt (2.0.11) - timeout (0.3.0) + strscan (3.1.0) + thor (1.3.1) + tilt (2.4.0) + timeout (0.4.1) turbolinks (5.2.1) turbolinks-source (~> 5.2) turbolinks-source (5.2.0) - tzinfo (2.0.5) + tzinfo (2.0.6) concurrent-ruby (~> 1.0) uglifier (4.2.0) execjs (>= 0.3.0, < 3) warden (1.2.9) rack (>= 2.0.9) - web-console (4.2.0) + web-console (4.2.1) actionview (>= 6.0.0) activemodel (>= 6.0.0) bindex (>= 0.4.0) railties (>= 6.0.0) - websocket (1.2.9) - websocket-driver (0.7.5) + websocket (1.2.11) + websocket-driver (0.7.6) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) xpath (3.2.0) nokogiri (~> 1.8) - zeitwerk (2.6.0) + zeitwerk (2.6.17) PLATFORMS - ruby + arm64-darwin + x86_64-darwin DEPENDENCIES byebug capybara coffee-rails + concurrent-ruby devise + factory_bot (~> 6.3) + factory_bot_rails (~> 6.2) + faker jbuilder + kaminari listen pg pry-rails puma rails (~> 7.0.3) + rails-controller-testing rspec-rails sass-rails selenium-webdriver + shoulda-matchers (~> 5.3) spring turbolinks tzinfo-data @@ -285,4 +322,4 @@ RUBY VERSION ruby 3.1.2p20 BUNDLED WITH - 2.3.22 + 2.5.17 diff --git a/README.md b/README.md index 500f71a1..4f2a1056 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# Top News: Internal Team News Feed +d# Top News: Internal Team News Feed In order to evaluate your stengths as a developer, we are requesting you complete a brief take-home code challenge that involves some work on the full web stack. We expect this to take 2 to 4 hours of your time. After developing your solution, please submit a Pull Request on Github and we will discuss your code on a screenshare at the next interview. @@ -25,3 +25,11 @@ When a team member signs in, they will see recent news stories and be able to st * As an internal tool for a small team, performance optimization is not a requirement. * Be prepared to discuss known performance shortcomings of your solution and potential improvements. * UX design here is of little importance. The design can be minimal or it can have zero design at all. + + +## Implementation Details + +* Most of the logics are in /app/controllers/stories_controller.rb +* By default, it looks up stories directly from database. +* A "Load Latest News" button for loading new stories from the API asynchronously to improve performance +* Tests created to cover happy paths. \ No newline at end of file diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index 46b20359..6897109c 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -1,15 +1,11 @@ -// This is a manifest file that'll be compiled into application.js, which will include all the files -// listed below. -// -// Any JavaScript/Coffee file within this directory, lib/assets/javascripts, or any plugin's -// vendor/assets/javascripts directory can be referenced here using a relative path. -// -// It's not advisable to add code directly here, but if you do, it'll appear at the bottom of the -// compiled file. JavaScript code in this file should be added after the last require_* statement. -// -// Read Sprockets README (https://github.com/rails/sprockets#sprockets-directives) for details -// about supported directives. -// -//= require rails-ujs -//= require turbolinks -//= require_tree . +document.addEventListener('DOMContentLoaded', function () { + var loadButton = document.getElementById('load-latest-news'); + var loadStatus = document.getElementById('load-status'); + + if (loadButton) { + loadButton.addEventListener('click', function () { + loadStatus.textContent = 'Loading...'; + loadStatus.style.display = 'inline'; + }); + } +}); \ No newline at end of file diff --git a/app/assets/stylesheets/application.css b/app/assets/stylesheets/application.css index d05ea0f5..0197293f 100644 --- a/app/assets/stylesheets/application.css +++ b/app/assets/stylesheets/application.css @@ -1,15 +1,31 @@ -/* - * This is a manifest file that'll be compiled into application.css, which will include all the files - * listed below. - * - * Any CSS and SCSS file within this directory, lib/assets/stylesheets, or any plugin's - * vendor/assets/stylesheets directory can be referenced here using a relative path. - * - * You're free to add application-wide styles to this file and they'll appear at the bottom of the - * compiled file so the styles you add here take precedence over styles defined in any other CSS/SCSS - * files in this directory. Styles in this file should be added after the last require_* statement. - * It is generally better to create a new file per style scope. - * - *= require_tree . - *= require_self - */ +nav { + background-color: #f8f9fa; + padding: 10px; + margin-bottom: 20px; +} + +nav a, +nav span, +nav input[type="submit"] { + margin-right: 10px; + text-decoration: none; + color: #007bff; +} + +nav input[type="submit"] { + background: none; + border: none; + cursor: pointer; + font-size: 1em; + color: #007bff; +} + +nav input[type="submit"]:hover { + text-decoration: underline; +} + +#load-status { + margin-left: 10px; + font-weight: bold; + color: #4CAF50; +} \ No newline at end of file diff --git a/app/assets/stylesheets/stories.css b/app/assets/stylesheets/stories.css new file mode 100644 index 00000000..02414b21 --- /dev/null +++ b/app/assets/stylesheets/stories.css @@ -0,0 +1,20 @@ +.story { + margin-bottom: 20px; + padding: 10px; + border: 1px solid #ddd; + border-radius: 5px; +} + +.starred { + color: gold; +} + +.unstarred { + color: #ccc; +} + +.star-button, +.unstar-button { + display: inline-block; + margin-right: 10px; +} \ No newline at end of file diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 1c07694e..39e2af5f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,3 +1,5 @@ class ApplicationController < ActionController::Base - protect_from_forgery with: :exception -end + def after_sign_out_path_for(resource_or_scope) + new_user_session_path + end +end \ No newline at end of file diff --git a/app/controllers/stories_controller.rb b/app/controllers/stories_controller.rb new file mode 100644 index 00000000..7971229e --- /dev/null +++ b/app/controllers/stories_controller.rb @@ -0,0 +1,87 @@ +require 'resolv-replace' +require 'concurrent' + +class StoriesController < ApplicationController + before_action :authenticate_user! + + def index + @stories = Story.order(created_at: :desc).page(params[:page]).per(20) + end + + def starred + @starred_stories = Story.joins(:starred_stories).distinct.order(created_at: :desc) + end + + def star + @story = Story.find_by(hacker_news_id: params[:id]) + current_user.star_story(@story) + respond_to do |format| + format.html { redirect_back(fallback_location: root_path) } + format.js + end + end + + def unstar + @story = Story.find_by(hacker_news_id: params[:id]) + current_user.unstar_story(@story) + respond_to do |format| + format.html { redirect_back(fallback_location: root_path) } + format.js + end + end + + def fetch_latest_news + # Fetch top story IDs concurrently + top_story_ids = fetch_top_story_ids || [] + new_story_ids = top_story_ids - Story.pluck(:hacker_news_id) + + puts "#{new_story_ids.count} new story id(s)" + + # Use concurrent-ruby to fetch story data concurrently + futures = new_story_ids.map do |id| + Concurrent::Future.execute { fetch_story_data(id) } + end + + # Wait for all futures to complete and get the results + futures.each do |future| + story_data = future.value + next unless story_data + + Story.create( + hacker_news_id: story_data['id'], + title: story_data['title'], + url: story_data['url'], + points: story_data['score'], + comments_count: story_data['descendants'] + ) + end + + @stories = Story.order(created_at: :desc).page(params[:page]).per(20) + respond_to do |format| + format.html { redirect_to root_path } + format.js + end + end + + private + + def fetch_top_story_ids + puts "Fetching top story ids via the API" + start_time = Time.now + top_story_ids = JSON.parse(Net::HTTP.get(URI('https://hacker-news.firebaseio.com/v0/topstories.json'))) + puts "Top story ids fetched, took #{Time.now - start_time} seconds" + top_story_ids + end + + def fetch_story_data(id) + puts "Fetching story with id #{id} via the API" + start_time = Time.now + story_data = JSON.parse(Net::HTTP.get(URI("#{story_url}#{id}.json"))) + puts "Story with id fetched, took #{Time.now - start_time} seconds" + story_data + end + + def story_url + 'https://hacker-news.firebaseio.com/v0/item/' + end + end \ No newline at end of file diff --git a/app/models/starred_story.rb b/app/models/starred_story.rb new file mode 100644 index 00000000..31469342 --- /dev/null +++ b/app/models/starred_story.rb @@ -0,0 +1,4 @@ +class StarredStory < ApplicationRecord + belongs_to :user + belongs_to :story +end diff --git a/app/models/story.rb b/app/models/story.rb new file mode 100644 index 00000000..48a0dc10 --- /dev/null +++ b/app/models/story.rb @@ -0,0 +1,9 @@ +class Story < ApplicationRecord + has_many :starred_stories + has_many :users, through: :starred_stories + + validates :hacker_news_id, presence: true, uniqueness: true + def starred_by_names + users.map(&:full_name).reject(&:blank?).join(', ') + end +end diff --git a/app/models/user.rb b/app/models/user.rb index b2091f9a..64f668de 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -3,4 +3,18 @@ class User < ApplicationRecord # :confirmable, :lockable, :timeoutable and :omniauthable devise :database_authenticatable, :registerable, :recoverable, :rememberable, :trackable, :validatable + has_many :starred_stories + has_many :stories, through: :starred_stories + + def star_story(story) + stories << story unless stories.include?(story) + end + + def unstar_story(story) + stories.delete(story) + end + + def full_name + "#{first_name} #{last_name}".strip + end end diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 331a7ed0..48609241 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -6,14 +6,24 @@ <%= csrf_meta_tags %> <%= stylesheet_link_tag 'application', media: 'all', 'data-turbolinks-track': 'reload' %> <%= javascript_include_tag 'application', 'data-turbolinks-track': 'reload' %> +
-- <%= notice %> -
-- <%= alert %> -
+ + +<%= notice %>
+<%= alert %>
+ <%= yield %> -