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

Apply fixes from StyleCI #1350

Merged
merged 1 commit into from
Jan 26, 2018
Merged

Apply fixes from StyleCI #1350

merged 1 commit into from
Jan 26, 2018

Conversation

tobyzerner
Copy link
Contributor

This pull request applies code style fixes from an analysis carried out by StyleCI.


For more information, click here.

[ci skip] [skip ci]
@tobyzerner tobyzerner merged commit f12a6f9 into visibility-scoping Jan 26, 2018
@tobyzerner tobyzerner deleted the analysis-XlLJ3V branch January 26, 2018 23:23
tobyzerner added a commit that referenced this pull request Jan 26, 2018
* Overhaul the way model visibility scoping works

- Previously post visibility scoping required concrete knowledge of the
  parent discussion, ie. you needed a Discussion model on which you
  would call `postsVisibleTo($actor)`. This meant that to fetch posts
  from different discussions (eg. when listing user posts), it was a
  convoluted process, ultimately causing #1333.

  Now posts behave like any other model in terms of visibility scoping,
  and you simply call `whereVisibleTo($actor)` on a Post query. This
  scope will automatically apply a WHERE EXISTS clause that scopes the
  query to only include posts whose discussions are visible too. Thus,
  fetching posts from multiple discussions can now be done in a single
  query, simplifying things greatly and fixing #1333.

- As such, the ScopePostVisibility event has been removed. Also, the
  rest of the "Scope" events have been consolidated into a single event,
  ScopeModelVisibility. This event is called whenever a user must have
  a certain $ability in order to see a set of discussions. Typically
  this ability is just "view". But in the case of discussions which have
  been marked as `is_private`, it is "viewPrivate". And in the case of
  discussions which have been hidden, it is "hide". etc.

  The relevant API on AbstractPolicy has been refined, now providing
  `find`, `findPrivate`, `findEmpty`, and `findWithPermission` methods.
  This could probably do with further refinement and we can re-address
  it once we get around to implementing more Extenders.

- An additional change is that Discussion::comments() (the relation
  used to calculate the cached number of replies) now yields "comments
  that are not private", where before it meant "comments that are
  visible to Guests". This was flawed because eg. comments in non-public
  tags are technically not visible to Guests.

  Consequently, the Approval extension must adopt usage of `is_private`,
  so that posts which are not approved are not included in the replies
  count. Fundamentally, `is_private` now indicates that a discussion/
  post should be hidden by default and should only be visible if it
  meets certain criteria. This is in comparison to non-is_private
  entities, which are visible by default and may be hidden if they don't
  meet certain criteria.

Note that these changes have not been extensively tested, but I have
been over the logic multiple times and it seems to check out.

* Add event to determine whether a discussion `is_private`

See #1153 (comment)

* Don't include hidden posts in the comments count

* Apply fixes from StyleCI (#1350)
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.

1 participant