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

[OFN Domains] Breaking OFN into domains - POC cookies inside an engine #2521

Merged
merged 10 commits into from
Oct 15, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ AllCops:
- !ruby/regexp /old_and_unused\.rb$/
# The parser gem fails to parse this file with out current Ruby version.
- 'spec/factories.rb'
# Excluding: inadequate Naming/FileName rule rejects GemFile name with camelcase
- 'engines/web/Gemfile'

# OFN SETTINGS
# Cop settings that have been agreed upon by the OFN community
Expand Down
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ gem 'i18n-js', '~> 3.0.0'
# Patched version. See http://rubysec.com/advisories/CVE-2015-5312/.
gem 'nokogiri', '>= 1.6.7.1'

gem 'web', path: './engines/web'

gem 'pg'
gem 'spree', github: 'openfoodfoundation/spree', branch: 'step-6a', ref: '69db1c090f3711088d84b524f1b94d25e6d21616'
gem 'spree_i18n', github: 'spree/spree_i18n', branch: '1-3-stable'
Expand Down
6 changes: 6 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,11 @@ GIT
activemodel (>= 3.0)
railties (>= 3.0)

PATH
remote: engines/web
specs:
web (0.0.1)

GEM
remote: https://rubygems.org/
specs:
Expand Down Expand Up @@ -767,6 +772,7 @@ DEPENDENCIES
capybara (>= 2.15.4)
coffee-rails (~> 3.2.1)
compass-rails
web!
custom_error_message!
daemons
dalli
Expand Down
1 change: 1 addition & 0 deletions app/assets/stylesheets/darkswarm/all.scss
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
@import 'base/*';
@import '*';
@import 'pages/*';
@import '../web/all';

ofn-modal {
display: block;
Expand Down
26 changes: 0 additions & 26 deletions app/controllers/api/cookies_consent_controller.rb

This file was deleted.

21 changes: 0 additions & 21 deletions app/helpers/cookies_policy_helper.rb

This file was deleted.

11 changes: 11 additions & 0 deletions app/helpers/footer_links_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
require 'web/cookies_consent'

module FooterLinksHelper
def cookies_policy_link
link_to( t( '.footer_data_cookies_policy' ), '', 'cookies-policy-modal' => true, 'cookies-banner' => !Web::CookiesConsent.new(cookies, request.host).exists? && Spree::Config.cookies_consent_banner_toggle)
end

def privacy_policy_link
link_to( t( '.footer_data_privacy_policy' ), Spree::Config.privacy_policy_url, target: '_blank' )
end
end
29 changes: 0 additions & 29 deletions app/services/cookies_consent.rb

This file was deleted.

1 change: 1 addition & 0 deletions app/views/layouts/darkswarm.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
%script{src: "//maps.googleapis.com/maps/api/js?libraries=places,geometry#{ ENV['GOOGLE_MAPS_API_KEY'] ? '&key=' + ENV['GOOGLE_MAPS_API_KEY'] : ''} "}
= stylesheet_link_tag "darkswarm/all"
= javascript_include_tag "darkswarm/all"
= javascript_include_tag "web/all"

= render "layouts/i18n_script"
= render "layouts/bugherd_script"
Expand Down
2 changes: 0 additions & 2 deletions app/views/shared/_footer.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,6 @@
= t '.footer_legal_text_html', {content_license: link_to('CC BY-SA 3.0', 'https://creativecommons.org/licenses/by-sa/3.0/'), code_license: link_to('AGPL 3', 'https://tldrlegal.com/license/gnu-affero-general-public-license-v3-(agpl-3.0)' )}
%p.text-small
%div
- cookies_policy_link = link_to( t( '.footer_data_cookies_policy' ), '', 'cookies-policy-modal' => true, 'cookies-banner' => !CookiesConsent.new(cookies, request.host).exists? && Spree::Config.cookies_consent_banner_toggle)
- privacy_policy_link = link_to( t( '.footer_data_privacy_policy' ), Spree::Config.privacy_policy_url, :target => '_blank' )
Matt-Yorkley marked this conversation as resolved.
Show resolved Hide resolved
- if Spree::Config.privacy_policy_url.present?
= t '.footer_data_text_with_privacy_policy_html', {cookies_policy: cookies_policy_link.html_safe, privacy_policy: privacy_policy_link.html_safe }
- else
Expand Down
7 changes: 3 additions & 4 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,17 +109,16 @@
get :job_queue
end

scope '/cookies' do
resource :consent, only: [:show, :create, :destroy], :controller => "cookies_consent"
end

resources :customers, only: [:index, :update]

post '/product_images/:product_id', to: 'product_images#update_product_image'
end

get 'sitemap.xml', to: 'sitemap#index', defaults: { format: 'xml' }

# Mount Web engine routes
mount Web::Engine, :at => '/'

# Mount Spree's routes
mount Spree::Core::Engine, :at => '/'
end
5 changes: 5 additions & 0 deletions engines/web/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Web

This is the rails engine for the Web domain.

See our wiki for [more info about domains and engines in OFN](https://github.com/openfoodfoundation/openfoodnetwork/wiki/Tech-Doc:-How-OFN-is-organized-in-Domains-using-Rails-Engines).
13 changes: 13 additions & 0 deletions engines/web/app/assets/javascripts/web/all.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// 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, vendor/assets/javascripts,
// or vendor/assets/javascripts of plugins, if any, 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
// the compiled file.
//
// WARNING: THE FIRST BLANK LINE MARKS THE END OF WHAT'S TO BE PROCESSED, ANY BLANK LINE SHOULD
// GO AFTER THE REQUIRES BELOW.
//
//= require_tree .
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Darkswarm.controller "CookiesBannerCtrl", ($scope, CookiesBannerService, $http, $window)->

$scope.acceptCookies = ->
$http.post('/api/cookies/consent')
$http.post('/web/api/cookies/consent')
CookiesBannerService.close()
CookiesBannerService.disable()
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Darkswarm.factory "CookiesBannerService", (Navigation, $modal, $location, Redire
modalMessage: null
isEnabled: false

open: (path, template = 'darkswarm/cookies_banner/cookies_banner.html') =>
open: (path, template = 'angular-templates/cookies_banner.html') =>
return unless @isEnabled
@modalInstance = $modal.open
templateUrl: template
Expand Down
2 changes: 2 additions & 0 deletions engines/web/app/assets/javascripts/web/web.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// Place all the behaviors and hooks related to the matching controller here.
// All this logic will automatically be available in application.js.
2 changes: 2 additions & 0 deletions engines/web/app/assets/stylesheets/web/all.css.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@import 'web/pages/cookies_banner';
@import 'web/pages/cookies_policy_modal';
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@import '../branding';
@import 'darkswarm/branding';

.cookies-banner {
background: $dark-grey;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@import '../branding';
@import 'darkswarm/branding';

.cookies-policy-modal {
background: $disabled-light;
Expand Down
30 changes: 30 additions & 0 deletions engines/web/app/controllers/web/api/cookies_consent_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
require_dependency 'web/cookies_consent'

module Web
module Api
class CookiesConsentController < BaseController
include ActionController::Cookies
respond_to :json

def show
render json: { cookies_consent: cookies_consent.exists? }
end

def create
cookies_consent.set
show
end

def destroy
cookies_consent.destroy
show
end

private

def cookies_consent
@cookies_consent ||= Web::CookiesConsent.new(cookies, request.host)
end
end
end
end
5 changes: 5 additions & 0 deletions engines/web/app/controllers/web/application_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module Web
class ApplicationController < ActionController::Base
Copy link
Contributor

@Matt-Yorkley Matt-Yorkley Sep 8, 2018

Choose a reason for hiding this comment

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

So is the purpose of this to allow access from within the engine to the helpers attached to the main ApplicationController? Wait, no... it's inheriting from ::Base, not our main ApplicationController. My mistake.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, this is a separate app, I dont think we will inherit from the main app app_controller. We want this app to be a different and separate app. We will need to see what we do what common code, but I dont think it should be through extending the main app_controller.

protect_from_forgery with: :exception
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this need to be overridden instead of using the regular ApplicationController value?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I guess I'm a bit confused about how this and the main ApplicationController will work together.

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 easiest way to think about an engine is to think of it as a separate app (from the guide "Engines can be considered miniature applications").
we can get to the engine code through requiring its modules and classes but the main way to get to engine will be through its routes (there's a new entry on the main app routes to include all the routes of this engine), so this is a different app and so we extend ActionController::Base.

Actually, this is a good example of where we will get the opportunity to better organize our code. If you look at the main ApplicationController you see it's contains code relate to embedded_shopfront code, this code can be moved to this Web module ApplicationController and removed from the main app and thus, cleaning up this code from the other spaces, like the unrelated backoffice code.

the reference for engines is this: https://guides.rubyonrails.org/engines.html

end
end
23 changes: 23 additions & 0 deletions engines/web/app/helpers/web/cookies_policy_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
module Web
module CookiesPolicyHelper
def render_cookie_entry(cookie_name, cookie_desc, cookie_domain = nil)
render partial: 'cookies_policy_entry',
locals: { cookie_name: cookie_name,
cookie_desc: cookie_desc,
cookie_domain: cookie_domain }
end

def matomo_iframe_src
"#{Spree::Config.matomo_url}"\
"/index.php?module=CoreAdminome&action=optOut"\
"&language=#{locale_language}"\
"&backgroundColor=&fontColor=222222&fontSize=16px&fontFamily=%22Roboto%22%2C%20Arial%2C%20sans-serif"
end

# removes country from locale if needed
# for example, both locales en and en_GB return language en
def locale_language
I18n.locale[0..1]
end
end
end
9 changes: 9 additions & 0 deletions engines/web/config/routes.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Web::Engine.routes.draw do
namespace :web do
namespace :api do
scope '/cookies' do
resource :consent, only: [:show, :create, :destroy], controller: "cookies_consent"
end
end
end
end
4 changes: 4 additions & 0 deletions engines/web/lib/web.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
require "web/engine"

module Web
end
31 changes: 31 additions & 0 deletions engines/web/lib/web/cookies_consent.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
module Web
class CookiesConsent
COOKIE_NAME = 'cookies_consent'.freeze

def initialize(cookies, domain)
@cookies = cookies
@domain = domain
end

def exists?
cookies.key?(COOKIE_NAME)
end

def destroy
cookies.delete(COOKIE_NAME, domain: domain)
end

def set
cookies[COOKIE_NAME] = {
value: COOKIE_NAME,
expires: 1.year.from_now,
domain: domain,
httponly: true
}
end

private

attr_reader :cookies, :domain
end
end
4 changes: 4 additions & 0 deletions engines/web/lib/web/engine.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
module Web
class Engine < ::Rails::Engine
end
end
3 changes: 3 additions & 0 deletions engines/web/lib/web/version.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module Web
VERSION = "0.0.1".freeze
end
52 changes: 52 additions & 0 deletions engines/web/spec/helpers/cookies_policy_helper_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
require 'spec_helper'

module Web
describe CookiesPolicyHelper, type: :helper do
# keeps global state unchanged
around do |example|
original_locale = I18n.locale
original_matomo_url = Spree::Config.matomo_url
example.run
Spree::Config.matomo_url = original_matomo_url
I18n.locale = original_locale
end

describe "matomo optout iframe src" do
describe "when matomo url is set" do
before do
Spree::Config.matomo_url = "http://matomo.org/"
end

scenario "includes the matomo URL" do
expect(helper.matomo_iframe_src).to include Spree::Config.matomo_url
end

scenario "is not equal to the matomo URL" do
expect(helper.matomo_iframe_src).to_not eq Spree::Config.matomo_url
end
end

scenario "is not nil, when matomo url is nil" do
Spree::Config.matomo_url = nil
expect(helper.matomo_iframe_src).to_not eq nil
end
end

describe "language from locale" do
scenario "when locale is the language" do
I18n.locale = "en"
expect(helper.locale_language).to eq "en"
end

scenario "is empty when locale is empty" do
I18n.locale = ""
expect(helper.locale_language).to be_empty
end

scenario "is only the language, when locale includes country" do
I18n.locale = "en_GB"
expect(helper.locale_language).to eq "en"
end
end
end
end
Loading