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

Allow :create Entry only for publishers on accounts, not on entries #836

Merged
merged 4 commits into from
Aug 28, 2017

Conversation

aviav
Copy link
Contributor

@aviav aviav commented Aug 13, 2017

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 99.06% when pulling 0c7019b on aviav:fix-entry-policy into 3d34806 on codevise:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 99.06% when pulling 0c7019b on aviav:fix-entry-policy into 3d34806 on codevise:master.

@tf tf modified the milestone: v12.1 Aug 14, 2017
@@ -87,7 +87,8 @@ def publish?
end

def create?
publish? && AccountPolicy::Scope.new(user, Account).entry_creatable.any?
query.has_at_least_account_role?(:publisher) &&
AccountPolicy::Scope.new(user, Account).entry_creatable.any?
Copy link
Member

@tf tf Aug 14, 2017

Choose a reason for hiding this comment

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

So the idea is that an entry publisher should not be able to create an entry? I agree with that. But shouldn't this spec fail then?

I don't really get the second part of the condition. Doesn't that say, that I'm allowed to create an entry if there is at least one account where i can create an entry (which also just means being publisher in at least one account)

@tf
Copy link
Member

tf commented Aug 16, 2017

Not sure why I am assigned here. What is the answer to the question?

@aviav
Copy link
Contributor Author

aviav commented Aug 16, 2017

I thought I understood that you wanted to look into those questions yourself. Maybe we need to clarify something here

to: :read,
topic: -> { create(:entry) }

it_behaves_like 'a membership-based permission that',
allows: :previewer,
of_entry: -> (topic) { topic },
of_account: -> (topic) { topic.account },
of_entry_or_its_account: -> (topic) { topic },

Choose a reason for hiding this comment

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

Do not use spaces between -> and opening brace in lambda literals

to: :preview,
topic: -> { create(:entry) }

it_behaves_like 'a membership-based permission that',
allows: :previewer,
of_entry: -> (topic) { topic },
of_account: -> (topic) { topic.account },
of_entry_or_its_account: -> (topic) { topic },

Choose a reason for hiding this comment

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

Do not use spaces between -> and opening brace in lambda literals

to: :confirm_encoding,
topic: -> { create(:entry) }

it_behaves_like 'a membership-based permission that',
allows: :previewer,
of_entry: -> (topic) { topic },
of_account: -> (topic) { topic.account },
of_entry_or_its_account: -> (topic) { topic },

Choose a reason for hiding this comment

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

Do not use spaces between -> and opening brace in lambda literals

to: :snapshot,
topic: -> { create(:entry) }

it_behaves_like 'a membership-based permission that',
allows: :editor,
but_forbids: :previewer,
of_entry: -> (topic) { topic },
of_account: -> (topic) { topic.account },
of_entry_or_its_account: -> (topic) { topic },

Choose a reason for hiding this comment

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

Do not use spaces between -> and opening brace in lambda literals

to: :restore,
topic: -> { create(:entry) }

it_behaves_like 'a membership-based permission that',
allows: :editor,
but_forbids: :previewer,
of_entry: -> (topic) { topic },
of_account: -> (topic) { topic.account },
of_entry_or_its_account: -> (topic) { topic },

Choose a reason for hiding this comment

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

Do not use spaces between -> and opening brace in lambda literals

to: :destroy_membership_on,
topic: -> { create(:entry) }

it_behaves_like 'a membership-based permission that',
allows: :publisher,
but_forbids: :editor,
of_entry: -> (topic) { topic },
of_account: -> (topic) { topic.account },
of_entry_or_its_account: -> (topic) { topic },

Choose a reason for hiding this comment

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

Do not use spaces between -> and opening brace in lambda literals

to: :edit_role_on,
topic: -> { create(:entry) }

it_behaves_like 'a membership-based permission that',
allows: :manager,
but_forbids: :publisher,
of_entry: -> (topic) { topic },
of_account: -> (topic) { topic.account },
of_entry_or_its_account: -> (topic) { topic },

Choose a reason for hiding this comment

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

Do not use spaces between -> and opening brace in lambda literals

to: :add_member_to,
topic: -> { create(:entry) }

it_behaves_like 'a membership-based permission that',
allows: :manager,
but_forbids: :publisher,
of_entry: -> (topic) { topic },
of_account: -> (topic) { topic.account },
of_entry_or_its_account: -> (topic) { topic },

Choose a reason for hiding this comment

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

Do not use spaces between -> and opening brace in lambda literals

to: :manage,
topic: -> { create(:entry) }

it_behaves_like 'a membership-based permission that',
allows: :manager,
but_forbids: :publisher,
of_entry: -> (topic) { topic },
of_account: -> (topic) { topic.account },
of_entry_or_its_account: -> (topic) { topic },

Choose a reason for hiding this comment

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

Do not use spaces between -> and opening brace in lambda literals

@@ -5,125 +5,115 @@ module Pageflow
it_behaves_like 'a membership-based permission that',
allows: :manager,
but_forbids: :publisher,
of_entry: -> (topic) { topic },
of_account: -> (topic) { topic.account },
of_entry_or_its_account: -> (topic) { topic },

Choose a reason for hiding this comment

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

Do not use spaces between -> and opening brace in lambda literals

tf and others added 3 commits August 22, 2017 10:51
Ensure specs for account and entry case can be generated in a single
invocation of the shared example. Prevent overriding `let` in a loop
causing only the last case to be used in specs.
* Fail if unknown option is passed.

* Allow using `of_entry_or_its_account` instead of passing both
  `of_entry` and `of_account` options to make specs more readable.

* Allow testing only the `forbid` case, by aliasing the `but_forbid`
  option and skipping specs for the `allow` case.

* Make spec description generation more DRY.
The previous implementation also allowed entry publishers as long as
they were also publishers of at least one (possibly unrelated)
account.
# `:of_account` params are present, the second iteration of the
# loop will override the definition of the first, causing the
# specs to be defined for the account case two times.
context do

Choose a reason for hiding this comment

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

Block has too many lines. [26/25]

let(:topic) { instance_exec(&params[:topic]) }
let(:entity) { of.call(topic) }
let(:policy) { described_class.new(user, topic) }
allowed_params = [

Choose a reason for hiding this comment

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

Use %i or %I for an array of symbols.

to: :read,
topic: -> { create(:entry) }

it_behaves_like 'a membership-based permission that',
allows: :previewer,
of_entry: -> (topic) { topic },
of_account: -> (topic) { topic.account },
of_entry_or_its_account: -> (topic) { topic },

Choose a reason for hiding this comment

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

Do not use spaces between -> and opening brace in lambda literals

to: :preview,
topic: -> { create(:entry) }

it_behaves_like 'a membership-based permission that',
allows: :previewer,
of_entry: -> (topic) { topic },
of_account: -> (topic) { topic.account },
of_entry_or_its_account: -> (topic) { topic },

Choose a reason for hiding this comment

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

Do not use spaces between -> and opening brace in lambda literals

to: :snapshot,
topic: -> { create(:entry) }

it_behaves_like 'a membership-based permission that',
allows: :editor,
but_forbids: :previewer,
of_entry: -> (topic) { topic },
of_account: -> (topic) { topic.account },
of_entry_or_its_account: -> (topic) { topic },

Choose a reason for hiding this comment

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

Do not use spaces between -> and opening brace in lambda literals

to: :index_widgets_for,
topic: -> { create(:entry) }

it_behaves_like 'a membership-based permission that',
allows: :editor,
but_forbids: :previewer,
of_entry: -> (topic) { topic },
of_account: -> (topic) { topic.account },
of_entry_or_its_account: -> (topic) { topic },

Choose a reason for hiding this comment

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

Do not use spaces between -> and opening brace in lambda literals

to: :edit,
topic: -> { create(:entry) }

it_behaves_like 'a membership-based permission that',
allows: :editor,
but_forbids: :previewer,
of_entry: -> (topic) { topic },
of_account: -> (topic) { topic.account },
of_entry_or_its_account: -> (topic) { topic },

Choose a reason for hiding this comment

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

Do not use spaces between -> and opening brace in lambda literals

allows: :publisher,
but_forbids: :editor,
of_entry: -> (topic) { topic },
of_account: -> (topic) { topic.account },
of_entry_or_its_account: -> (topic) { topic },

Choose a reason for hiding this comment

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

Do not use spaces between -> and opening brace in lambda literals

to: :edit_role_on,
topic: -> { create(:entry) }

it_behaves_like 'a membership-based permission that',
allows: :manager,
but_forbids: :publisher,
of_entry: -> (topic) { topic },
of_account: -> (topic) { topic.account },
of_entry_or_its_account: -> (topic) { topic },

Choose a reason for hiding this comment

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

Do not use spaces between -> and opening brace in lambda literals

to: :add_member_to,
topic: -> { create(:entry) }

it_behaves_like 'a membership-based permission that',
allows: :manager,
but_forbids: :publisher,
of_entry: -> (topic) { topic },
of_account: -> (topic) { topic.account },
of_entry_or_its_account: -> (topic) { topic },

Choose a reason for hiding this comment

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

Do not use spaces between -> and opening brace in lambda literals

@coveralls
Copy link

coveralls commented Aug 22, 2017

Coverage Status

Coverage increased (+0.1%) to 99.167% when pulling 8b8d2a9 on aviav:fix-entry-policy into ffc8da5 on codevise:master.

@coveralls
Copy link

coveralls commented Aug 22, 2017

Coverage Status

Coverage increased (+0.1%) to 99.167% when pulling 8b8d2a9 on aviav:fix-entry-policy into ffc8da5 on codevise:master.

@tf tf assigned tf and unassigned aviav Aug 28, 2017
@coveralls
Copy link

coveralls commented Aug 28, 2017

Coverage Status

Coverage increased (+0.1%) to 99.137% when pulling b130927 on aviav:fix-entry-policy into 5f21f8e on codevise:master.

@tf tf merged commit 2ef9fd9 into codevise:master Aug 28, 2017
tilsammans added a commit to scrollytelling/app that referenced this pull request Nov 12, 2017
2017-11-07

[Compare changes](codevise/pageflow@12-0-stable...v12.1.0)

- Include HLS in output presences of legacy files
  ([#872](codevise/pageflow#872),
   [#867](codevise/pageflow#867))

  The database migration for Pageflow 12.0 which updates output
  presences of existing video files is missing the HLS variant. This
  causes HLS urls of existing video files to be rendered as
  `undefined`.

  To apply the fix, install migrations and migrate your database. This
  fix has previously been released as part of version 12.0.2.

- Add Resque::Server to the generated routes
  ([#871](codevise/pageflow#871))

  Mounting the Resque web server makes it easier to inspect background
  workers and restart jobs that have failed. See the issue description
  of [#856](codevise/pageflow#856) on how to
  add this to existing apps.

- The theme configured on account level now only acts as a default for
  new entries. After enabling the `selectable_themes` feature, a theme
  selection dialog is available inside the editor from the "Title and
  Options > Appearance" tab. The dialog allows configuring the theme
  on a per revision basis.
  ([#781](codevise/pageflow#781),
   [#897](codevise/pageflow#897))

- Use page from url hash es landing page
  ([#832](codevise/pageflow#832))
- Do not record history when changing pages via scrolling
  ([#831](codevise/pageflow#831))
- Improve text tracks and info box display logic
  ([#826](codevise/pageflow#826))
- Bug fix: Fix order of public i18n fallback
  ([#883](codevise/pageflow#883))
- Bug fix: Prevent display of NaN duration in video controls
  ([#878](codevise/pageflow#878))
- Bug fix: Prevent 404 when share image has been deleted
  ([#816](codevise/pageflow#816))

- Use searchable select boxes in admin forms
  ([#888](codevise/pageflow#888))
- Remove sensitive data from active admin downloads
  ([#899](codevise/pageflow#899))
- Add config option to prevent multi account users
  ([#848](codevise/pageflow#848),
   [#868](codevise/pageflow#868))
- Add config options to restrict account manager permissions
  ([#849](codevise/pageflow#849))
- Fix N+1 query in account admin users tab
  ([#877](codevise/pageflow#877))
- Bug fix: Hide restore link and snapshot button for entry previewers
  ([#853](codevise/pageflow#853))
- Bug fix: Use copy of current_user in profile form
  ([#850](codevise/pageflow#850))
- Bug fix: Ensure new entry button is hidden for editors
  ([#847](codevise/pageflow#847))
- Bug fix: Show folder edit button only for publishers and above
  ([#838](codevise/pageflow#838))
- Bug fix: Allow :create entry only for publishers on accounts, not on entries
  ([#836](codevise/pageflow#836))

- Widget configuration
  ([#694](codevise/pageflow#694),
   [#809](codevise/pageflow#809))
- Bug fix: Ensure page order in editor preview stays up to date
  ([#898](codevise/pageflow#898))
- Bug fix: Switch off file meta data edit links when uploading
  ([#857](codevise/pageflow#857))
- Bug fix: Improve polling for HostedFile state
  ([#822](codevise/pageflow#822))
- Bug fix: Handle undefined page title.
  ([#763](codevise/pageflow#763))

- Use relative urls inside dash and hls playlists
  ([#842](codevise/pageflow#842))
- Use web audio api for volume fading if available
  ([#800](codevise/pageflow#800),
   [#863](codevise/pageflow#863))

- Add panorama_mask image file style
  ([#830](codevise/pageflow#830))
- Bug fix: Make url template generation more robust
  ([#876](codevise/pageflow#876))

- Themes can now be guarded by feature flags
  ([#765](codevise/pageflow#765))
- Extend EncodingConfirmation by public account attribute
  ([#817](codevise/pageflow#817))
- Extend query interface
  ([#815](codevise/pageflow#815))
- Accept options for accounts admin menu via config
  ([#811](codevise/pageflow#811))
- Add placeholder options to textareainputview
  ([#807](codevise/pageflow#807))
- Call hook on entry publication
  ([#806](codevise/pageflow#806))
- Add rake task and resque job to delete old auto snapshots
  ([#861](codevise/pageflow#861),
   [#882](codevise/pageflow#882))
- Generate a secure password in the seeds
  ([#775](codevise/pageflow#775))

- Allow specifying opacity of image variant logo
  ([#799](codevise/pageflow#799))
- Allow setting size of custom loading spinner background
  ([#798](codevise/pageflow#798))
- Add theme option to right align logo in desktop layout
  ([#797](codevise/pageflow#797))
- Add theme option to hide scroll indicator
  ([#790](codevise/pageflow#790))
- Make content text max width configurable
  ([#780](codevise/pageflow#780),
   [#804](codevise/pageflow#804))

- Import nginx-upload-module guide from wiki
  ([#814](codevise/pageflow#814),
   [#821](codevise/pageflow#821))
- Add documentation for versioning policy
  ([#866](codevise/pageflow#866))
- Fix small typos
  ([#787](codevise/pageflow#787))

- Precompile assets in Travis run
  ([#873](codevise/pageflow#873))
- Test workflow improvements
  ([#803](codevise/pageflow#803))
- Generate admins with account membership in specs
  ([#840](codevise/pageflow#840))
- Use rails 4.2.9 in tests
  ([#837](codevise/pageflow#837))
- Clean up memberships admin code
  ([#835](codevise/pageflow#835))
- Basic tests for UsersTab add user button
  ([#805](codevise/pageflow#805))
- Use codevise/teaspoon for logging backtraces on console
  ([#786](codevise/pageflow#786))
- Split vendored code from our own
  ([#885](codevise/pageflow#885))
- Introduce ApplicationRecord
  ([#889](codevise/pageflow#889))
- Move configuration into a concern
  ([#794](codevise/pageflow#794))
- Read the attribute instead of super
  ([#810](codevise/pageflow#810))
- Destroy widgets when parent subject is destroyed
  ([#834](codevise/pageflow#834))
- Fix dummy app generation on Travis for Rubygems 2.7
  ([#893](codevise/pageflow#893))
- Update contents of gemspec
  ([#891](codevise/pageflow#891))
- Bug fix: Work around breaking change in resque_mailer 2.4.3
  ([#894](codevise/pageflow#894))

See
[12-0-stable](https://github.com/codevise/pageflow/blob/12-0-stable/CHANGELOG.md)
branch for previous changes.
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