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

Static accountability pages #124

Merged

Conversation

amaia
Copy link
Contributor

@amaia amaia commented May 24, 2017

🎩 What? Why?

These are the global static accountability pages

📌 Related Issues

📋 Pending tasks

  • I18n for all texts
  • Updated texts in spanish and catalan

@amaia amaia force-pushed the static-accountability-pages branch from 83a3787 to e5770b9 Compare May 24, 2017 14:40
Copy link
Contributor

@josepjaume josepjaume left a comment

Choose a reason for hiding this comment

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

Approved but with minor comments. Good work!

private

def current_organization
@current_organization ||= request.env["decidim.current_organization"]
Copy link
Contributor

Choose a reason for hiding this comment

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

You could just include:

include Decidim::NeedsOrganization

And this will both ensure you won't access this without an organization and include the current_organization as a helper.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@josepjaume thanks! I'll use that instead

@current_organization ||= request.env["decidim.current_organization"]
end

def decidim_page_title
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this left stray? What about I18n?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@josepjaume oops, I forgot about this one, I'll fix it now

@@ -0,0 +1,2 @@
class Ability < Decidim::Ability
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this file needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@josepjaume it breaks in development when I remove this file, but I don't know why

ActionView::Template::Error (uninitialized constant Ability):
    1: <li><%= link_to t(".profile"), decidim.account_path %></li>
    2: <% if can? :read, :admin_dashboard %>
    3:   <li><%= link_to t(".admin_dashboard"), decidim_admin.root_path %></li>
    4: <% end %>
    5: <li><%= link_to t(".sign_out"), decidim.destroy_user_session_path, method: :delete, class: "sign-out-link" %></li>

cancancan (1.16.0) lib/cancan/controller_additions.rb:371:in `current_ability'
cancancan (1.16.0) lib/cancan/controller_additions.rb:394:in `can?'
actionpack (5.0.2) lib/abstract_controller/helpers.rb:68:in `can?'
/Users/amaia/.rbenv/versions/2.4.1/lib/ruby/gems/2.4.0/bundler/gems/decidim-c400beaf7655/decidim-core/app/views/layouts/decidim/_user_menu.html.erb:2:in `___sers_amaia__rbenv_versions_______lib_ruby_gems_______bundler_gems_decidim_c___beaf_____decidim_core_app_views_layouts_decidim__user_menu_html_erb__3410750993160859078_70276087318260'

@oriolgual oriolgual merged commit 9eff191 into AjuntamentdeBarcelona:master May 29, 2017
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