Fixes bug - Owners can now add sites to projects

Fixes #328

Also:
- updates permssions to match specification document from wiki (*/new
should be accessible for all routes).
- adds a :create_sites helper to Project abilities
- explicitly defines permissions for Project :update_sites
- Fixes broken unit test

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 ## Unreleased
 
+- 2017-02-24
+    - Fixed bugs with site/project permissions - owners can once again create new sites. Further updated site new 
+    permissions to match specification.
+    See [#328](https://github.com/QutBioacoustics/baw-server/issues/328)
+
 ## [Release 1.2.1](https://github.com/QutBioacoustics/baw-server/releases/tag/1.2.1)
 
  - 2017-02-20

app/models/ability.rb

@@ -166,12 +166,19 @@ def to_project(user, is_guest)
       Access::Core.can?(user, :reader, project)
     end
 
-    # TODO: only admin can #edit_sites, #update_sites (update_sites is html-only)
+    # only admin can #edit_sites, #update_sites (update_sites is html-only)
+    # below definition is redundant but added for clarity
+    can [:update_sites], Project do |project|
+      check_model(project)
+      Access::Core.is_admin?(user)
+    end
+
     # TODO: update_sites will be merged with sites#orphans
 
     # must be owner to do these actions
     # :update_permissions is html-only
-    can [:edit, :update, :update_permissions, :destroy], Project do |project|
+    # :creates_sites:  Can we create a site that belongs to this project? This is mainly used in views
+    can [:edit, :update, :update_permissions, :destroy, :create_sites], Project do |project|
       check_model(project)
       Access::Core.can?(user, :owner, project)
     end
@@ -233,22 +240,18 @@ def to_site(user, is_guest)
       Access::Core.can_any?(user, :reader, site.projects)
     end
 
-    # must have write permission or higher to create a new site in a project (logged in users only as guest users can only be reader)
-    can [:create, :new], Site do |site|
-      check_model(site)
-      Access::Core.check_orphan_site!(site)
-      Access::Core.can_any?(user, :writer, site.projects)
-    end
-
-    # only owner can access these actions
-    can [:edit, :update, :destroy, :upload_instructions, :harvest], Site do |site|
+    # only owner can access these actions.
+    # :create (create a new site in a project) requires an instance to be available before permissions are checked.
+    # This is done in controller action.
+    # Note: duplicate definition :create_sites in project abilities above used for rendering view capabilities.
+    can [:create, :edit, :update, :destroy, :upload_instructions, :harvest], Site do |site|
       check_model(site)
       Access::Core.check_orphan_site!(site)
       Access::Core.can_any?(user, :owner, site.projects)
     end
 
     # available to any user, including guest
-    can [:index, :filter], Site
+    can [:index, :filter, :new], Site
   end
 
   def to_audio_recording(user, is_guest)

app/views/projects/show.html.haml

@@ -12,8 +12,9 @@
     = edit_link(edit_project_path(@project), 'project')
   - if can? :update_permissions, @project
     = edit_link(project_permissions_path(@project), 'permissions', 'key')
-  - if can?(:update_sites, @project)
+  - if can? :create_sites, @project
     = new_link(new_project_site_path(@project), 'site')
+  - if can?(:update_sites, @project)
     = nav_item(href: edit_sites_project_path(@project),
         title: t('baw.shared.links.site_mapping.title'),
         tooltip: t('baw.shared.links.site_mapping.description') )

spec/acceptance/sites_spec.rb

@@ -113,13 +113,13 @@ def sites_body_params
   get '/projects/:project_id/sites/new' do
     let(:project_id) { project.id }
     let(:authentication_token) { reader_token }
-    standard_request_options(:get, 'NEW (as reader)', :forbidden, {expected_json_path: get_json_error_path(:permissions)})
+    standard_request_options(:get, 'NEW (as reader)',  :ok, {expected_json_path: 'data/longitude'})
   end
 
   get '/projects/:project_id/sites/new' do
     let(:project_id) { project.id }
     let(:authentication_token) { no_access_token }
-    standard_request_options(:get, 'NEW (as no access user)', :forbidden, {expected_json_path: get_json_error_path(:permissions)})
+    standard_request_options(:get, 'NEW (as no access user)',  :ok, {expected_json_path: 'data/longitude'})
   end
 
   get '/projects/:project_id/sites/new' do
@@ -130,7 +130,7 @@ def sites_body_params
 
   get '/projects/:project_id/sites/new' do
     let(:project_id) { project.id }
-    standard_request_options(:get, 'NEW (as anonymous user)', :unauthorized, {remove_auth: true, expected_json_path: get_json_error_path(:sign_up)})
+    standard_request_options(:get, 'NEW (as anonymous user)',  :ok, {expected_json_path: 'data/longitude'})
   end
 
   ################################
@@ -160,7 +160,7 @@ def sites_body_params
     let(:project_id) { project.id }
     let(:authentication_token) { writer_token }
     let(:raw_post) { {site: post_attributes}.to_json }
-    standard_request_options(:post, 'CREATE (as writer)', :created, {expected_json_path: 'data/project_ids'})
+    standard_request_options(:post, 'CREATE (as writer)', :forbidden, {expected_json_path: get_json_error_path(:permissions)})
   end
 
   post '/projects/:project_id/sites' do

spec/features/projects_spec.rb

@@ -366,7 +366,7 @@ def select_by_value(id, value)
         visit project_path(project)
         expect(page).to have_content(project.name)
         expect(page).to have_link('Edit this project')
-        expect(page).not_to have_link('New site')
+        expect(page).to have_link('New site')
         expect(page).to have_link('Edit permissions')
         expect(page).not_to have_link('Assign sites')
         expect(page).to have_button('Delete this project')

spec/features/sites_spec.rb

@@ -127,8 +127,20 @@
     expect(page).not_to have_button('Delete this site')
   end
 
-  it 'rejects access to create project site' do
+  it 'allows access to create project site' do
     visit new_project_site_path(project)
+    expect(page).to have_content('New Site')
+  end
+
+  # we allow access to new for API (so form as well) but we don't allow the form to work
+  it 'rejects a new site when filling out form correctly' do
+    url = new_project_site_path(project)
+    visit url
+    #save_and_open_page
+    fill_in 'site[name]', with: 'test name'
+    fill_in 'site[description]', with: 'description'
+    attach_file('site[image]', 'public/images/user/user-512.png')
+    click_button 'Submit'
     expect(page).to have_content(I18n.t('devise.failure.unauthorized'))
   end
 
@@ -167,8 +179,20 @@
 
   end
 
-  it 'rejects access to create project site' do
+  it 'allows access to create project site' do
     visit new_project_site_path(project)
+    expect(page).to have_content('New Site')
+  end
+
+  # we allow access to new for API (so form as well) but we don't allow the form to work
+  it 'rejects a new site when filling out form correctly' do
+    url = new_project_site_path(project)
+    visit url
+    #save_and_open_page
+    fill_in 'site[name]', with: 'test name'
+    fill_in 'site[description]', with: 'description'
+    attach_file('site[image]', 'public/images/user/user-512.png')
+    click_button 'Submit'
     expect(page).to have_content(I18n.t('devise.failure.unauthorized'))
   end