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

Enable setting a customized user guide link in general settings #2686

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
27 changes: 18 additions & 9 deletions app/controllers/admin/contents_controller.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,9 @@
module Admin
class ContentsController < Spree::Admin::BaseController
def edit
@preference_sections = [{name: I18n.t('admin.contents.edit.header'), preferences: [:logo, :logo_mobile, :logo_mobile_svg]},
{name: I18n.t('admin.contents.edit.home_page'), preferences: [:home_hero, :home_show_stats]},
{name: I18n.t('admin.contents.edit.producer_signup_page'), preferences: [:producer_signup_pricing_table_html, :producer_signup_case_studies_html, :producer_signup_detail_html]},
{name: I18n.t('admin.contents.edit.hub_signup_page'), preferences: [:hub_signup_pricing_table_html, :hub_signup_case_studies_html, :hub_signup_detail_html]},
{name: I18n.t('admin.contents.edit.group_signup_page'), preferences: [:group_signup_pricing_table_html, :group_signup_case_studies_html, :group_signup_detail_html]},
{name: I18n.t('admin.contents.edit.main_links'), preferences: [:menu_1, :menu_1_icon_name, :menu_2, :menu_2_icon_name, :menu_3, :menu_3_icon_name, :menu_4, :menu_4_icon_name, :menu_5, :menu_5_icon_name, :menu_6, :menu_6_icon_name, :menu_7, :menu_7_icon_name]},
{name: I18n.t('admin.contents.edit.footer_and_external_links'), preferences: [:footer_logo,
:footer_facebook_url, :footer_twitter_url, :footer_instagram_url, :footer_linkedin_url, :footer_googleplus_url, :footer_pinterest_url,
:footer_email, :community_forum_url, :footer_links_md, :footer_about_url]}]
@preference_sections = preference_sections.map do |preference_section|
{ name: preference_section.name, preferences: preference_section.preferences }
end
end

def update
Expand All @@ -26,5 +20,20 @@ def update

redirect_to main_app.edit_admin_content_path
end

private

def preference_sections
[
PreferenceSections::HeaderSection.new,
PreferenceSections::HomePageSection.new,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mkllnk something like this ?

Copy link
Member

Choose a reason for hiding this comment

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

Yes. I probably would have omitted the new here and called it somewhere else to reduce code duplication and separate the two concerns "list the sections" and "initialise a section to use it". But I'm happy with this.

PreferenceSections::ProducerSignupPageSection.new,
PreferenceSections::HubSignupPageSection.new,
PreferenceSections::GroupSignupPageSection.new,
PreferenceSections::MainLinksSection.new,
PreferenceSections::FooterAndExternalLinksSection.new,
PreferenceSections::UserGuideSection.new
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this misses a bit the point and violates Open-closed principle (sorry, I think the buzzword is relevant in this context), but it is still better than it was, so 👍

Copy link
Member

Choose a reason for hiding this comment

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

Okay so it should be open to extension and closed to change, right?

View one

Closed to change means that I'm not allowed to change a file. Adding a file is an extension and good. So we just have files following a convention and auto-load them.

View two

Classes, files, directories are all data structures. Adding a section is just adding a line here. That's extension, not modification. And if you add a file to a directory, you modify that directory as well. In the end, we want to change the list of sections here and insert one particular section at a certain position. This code makes that really clear.

]
end
end
end
3 changes: 3 additions & 0 deletions app/models/content_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,7 @@ class ContentConfiguration < Spree::Preferences::FileConfiguration
EOS

preference :footer_about_url, :string, default: "http://www.openfoodnetwork.org/ofn-local/open-food-network-australia/"

#User Guide
preference :user_guide_link, :string, default: 'https://guide.openfoodnetwork.org/'
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
module PreferenceSections
class FooterAndExternalLinksSection
def name
I18n.t('admin.contents.edit.footer_and_external_links')
end

def preferences
[
:footer_logo,
:footer_facebook_url,
:footer_twitter_url,
:footer_instagram_url,
:footer_linkedin_url,
:footer_googleplus_url,
:footer_pinterest_url,
:footer_email,
:community_forum_url,
:footer_links_md,
:footer_about_url
]
end
end
end
15 changes: 15 additions & 0 deletions app/models/preference_sections/group_signup_page_section.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
module PreferenceSections
class GroupSignupPageSection
def name
I18n.t('admin.contents.edit.group_signup_page')
end

def preferences
[
:group_signup_pricing_table_html,
:group_signup_case_studies_html,
:group_signup_detail_html
]
end
end
end
15 changes: 15 additions & 0 deletions app/models/preference_sections/header_section.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
module PreferenceSections
class HeaderSection
def name
I18n.t('admin.contents.edit.header')
end

def preferences
[
:logo,
:logo_mobile,
:logo_mobile_svg
]
end
end
end
14 changes: 14 additions & 0 deletions app/models/preference_sections/home_page_section.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
module PreferenceSections
class HomePageSection
def name
I18n.t('admin.contents.edit.home_page')
end

def preferences
[
:home_hero,
:home_show_stats
]
end
end
end
15 changes: 15 additions & 0 deletions app/models/preference_sections/hub_signup_page_section.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
module PreferenceSections
class HubSignupPageSection
def name
I18n.t('admin.contents.edit.hub_signup_page')
end

def preferences
[
:hub_signup_pricing_table_html,
:hub_signup_case_studies_html,
:hub_signup_detail_html
]
end
end
end
26 changes: 26 additions & 0 deletions app/models/preference_sections/main_links_section.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
module PreferenceSections
class MainLinksSection
def name
I18n.t('admin.contents.edit.main_links')
end

def preferences
[
:menu_1,
:menu_1_icon_name,
:menu_2,
:menu_2_icon_name,
:menu_3,
:menu_3_icon_name,
:menu_4,
:menu_4_icon_name,
:menu_5,
:menu_5_icon_name,
:menu_6,
:menu_6_icon_name,
:menu_7,
:menu_7_icon_name
]
end
end
end
15 changes: 15 additions & 0 deletions app/models/preference_sections/producer_signup_page_section.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
module PreferenceSections
class ProducerSignupPageSection
def name
I18n.t('admin.contents.edit.producer_signup_page')
end

def preferences
[
:producer_signup_pricing_table_html,
:producer_signup_case_studies_html,
:producer_signup_detail_html
]
end
end
end
11 changes: 11 additions & 0 deletions app/models/preference_sections/user_guide_section.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module PreferenceSections
class UserGuideSection
def name
I18n.t('admin.contents.edit.user_guide')
end

def preferences
[:user_guide_link]
end
end
end
2 changes: 1 addition & 1 deletion app/views/admin/shared/_user_guide_link.html.haml
Original file line number Diff line number Diff line change
@@ -1 +1 @@
= button_link_to t('.user_guide'), "http://www.openfoodnetwork.org/platform/user-guide/", icon: 'icon-external-link', target: '_blank'
= button_link_to t('.user_guide'), ContentConfig.user_guide_link, icon: 'icon-external-link', target: '_blank'
2 changes: 1 addition & 1 deletion app/views/enterprise_mailer/welcome.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
= "#{t(:email_registered)} #{ Spree::Config.site_name }!"

%p
= t :email_userguide_html, link: link_to('Open Food Network User Guide', 'http://www.openfoodnetwork.org/platform/user-guide/')
= t :email_userguide_html, link: link_to('Open Food Network User Guide', ContentConfig.user_guide_link)

%p
= t :email_admin_html, link: link_to('Admin Panel', spree.admin_url)
Expand Down
3 changes: 3 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,7 @@ en:
main_links: Main Menu Links
footer_and_external_links: Footer and External Links
your_content: Your content
user_guide: User Guide

enterprise_fees:
index:
Expand Down Expand Up @@ -1200,6 +1201,8 @@ en:
footer_links_md: "Links"
footer_about_url: "About URL"

user_guide_link: "User Guide Link"

name: Name
first_name: First Name
last_name: Last Name
Expand Down
24 changes: 18 additions & 6 deletions spec/features/admin/content_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,35 @@
fill_in 'footer_twitter_url', with: 'http://twitter.com/me'
fill_in 'footer_links_md', with: '[markdown link](/)'
click_button 'Update'
page.should have_content 'Your content has been successfully updated!'
expect(page).to have_content 'Your content has been successfully updated!'

visit root_path

# Then social media icons are only shown if they have a value
page.should_not have_selector 'i.ofn-i_044-facebook'
page.should have_selector 'i.ofn-i_041-twitter'
expect(page).not_to have_selector 'i.ofn-i_044-facebook'
expect(page).to have_selector 'i.ofn-i_041-twitter'

# And markdown is rendered
page.should have_link 'markdown link'
expect(page).to have_link 'markdown link'
end

scenario "uploading logos" do
attach_file 'logo', "#{Rails.root}/app/assets/images/logo-white.png"
click_button 'Update'
page.should have_content 'Your content has been successfully updated!'
expect(page).to have_content 'Your content has been successfully updated!'

ContentConfig.logo.to_s.should include "logo-white"
expect(ContentConfig.logo.to_s).to include "logo-white"
end

scenario "setting user guide link" do
fill_in 'user_guide_link', with: 'http://www.openfoodnetwork.org/platform/user-guide/'
click_button 'Update'

expect(page).to have_content 'Your content has been successfully updated!'

visit spree.admin_path

expect(page).to have_link('User Guide', href: 'http://www.openfoodnetwork.org/platform/user-guide/')
expect(find_link('User Guide')[:target]).to eq('_blank')
end
end