```shell
❯ rg "@notifications_index"
```
The above shell command exits status code 1, which means there are no
results found.
This relates to exploratory work for forem/forem#16821
* Don't suggest users for an anonymous visitor to follow
The sidebar raises an error when rendering suggested users for a tag
when the user is signed out.
https://app.honeybadger.io/projects/66984/faults/79342962/
* Assert empty response returned for anonymous requests
As part of removing Stackbit integration, I previously missed removing
the translation key. This removes an errant key.
This commit was driven by attempting to address the following feature
request:
> Conditional hide "Generate a personal blog from your DEV Community
> posts" in /settings/extensions
There is option for this behavior.
Closes#16819
Missed as part of #15701
* Authorize Web Monetization If User Can Create Article
There are three things I'm introducing in this PR:
1. Extracting a partial
2. Reworking the i18n keys
3. Adding a policy check regarding Web Monetization
In *extracting a partial*, I'm following the existing pattern where
other extensions have their own partial.
In *reworking the i18n keys*, I'm ensuring that the keys are part of the
same namespace. This will make finding their usage easier. Further, if
we decide to remove (or convert to a plugin) the web_monetization, then
we're just a bit closer to that possibility.
Last, and the reason for the work, is *Adding a policy check regarding
Web Monetization*. This follows on the work in forem/forem#16790.
Closes forem/forem#16820
Related to forem/forem#15098
There are two things to test:
1. Does the feature flag work or not.
2. Are the i18n keys properly applied.
For the feature flag:
- checkout this branch
- in rails console `FeatureFlag.enable(:limit_post_creation_to_admins)`
- start the rails server
- login as a non-admin user
- go to /settings/extensions and scroll to the bottom, you **shouldn't** see
the partial
- login as an admin user
- go to /settings/extensions and scroll to the bottom, you **should** see
the partial
Or visually verify the relatively simple change (and accept that it
conforms to #16790's existing pattern).
For the i18n keys, I have a before screenshot (from DEV.to) and the
after (from the changes on this branch).
* Adjustments based on contributor feedback
* Apply overflow-wrap fallback everywhere we use anywhere value
* fix some more non breaking spaces layout issues
* user with org sidebar
* comment index header
We're seeing these fail intermittently (typing the color doesn't
result in the desired color in the form), and will disable temporarily
to avoid unrelated build failures.
This commit does three things:
1) Adds and tests the ArticlePolicy#moderate? feature
2) Refactors the spec helpers to be a bit more flexible
3) Adds a few more edge case tests around ArticlePolicy
My apologies for conflating these changes, as it makes this commit
more challenging to review.
The goal is to adequate demonstrate the logic around users who can or
cannot moderate. At present, the rules for who can moderate is here:
841491c6ee/app/javascript/packs/articleModerationTools.js (L17-L27)
```js
if (user?.trusted) {
if (user?.id !== articleAuthorId && !isModerationPage()) {
initializeActionsPanel(user, path);
initializeFlagUserModal(articleAuthorId);
// "/mod" page
} else if (isModerationPage()) {
initializeActionsPanel(user, path);
initializeFlagUserModal(articleAuthorId);
}
}
```
- Related to forem/forem#16783
- Closesforem/forem#16784
Videos are a conceptual subset of Articles (see
[VideosController#create][1] for supporting "evidence"). As such, we
want to ensure they conform to the expectations of the ArticlePolicy (as
described in forem/forem#16483).
This feature enhancement does a few things:
1. Renames swaps the `VideoPolicy#new?` and `VideoPolicy#create?`
declarations (reversing the alias direction)
2. Moves methods out of the concrete ArticlesPolicy into the
ApplicationPolicy.
3. Exposes these user oriented methods for external
consumers (e.g. making future refactoring work easier). The happy
benefit is that this already removes a bit of duplicated logic.
4. Highlights that while yes the Policy is correct, we also may be
flirting with the concept of some sort of User "properties as it relates
to authorization" logic (good standing users, established users,
etc.)
5. Reworked the timing of a guard clause and setting of instance variables.
[1]:
33e9bac0f7/app/controllers/videos_controller.rb (L18-L22))
Relates to #16483Closesforem/forem#16728
To test this, I'm relying on the existing test suite. I envision that
shifting from returning false for suspended users to raising an
exception might cause some false positive errors. Note, in the
[AppliciationController][2] we handle the responses. Paired with
[config/appliciation.rb][3], we handle the policy exceptions.
[2]:33e9bac0f7/app/controllers/application_controller.rb (L32)
[3]:33e9bac0f7/config/application.rb (L71-L81)
With this commit, we're only allowing users who can create articles to
enter RSS feed information for fetching of articles.
Note: I chose not to indent body of the if conditional to ease the code
review. I also chose to place the if statement in this partial instead
of in the [app/views/users/_extensions.html.erb][1] file; this helps
contain the logic around the policy.
For sleuthing this mirrors the approach of forem/forem#16735Closesforem/forem#16788
Related to forem/forem#16766, forem/forem#16732, and forem/forem#16763
[1]:b87fd77992/app/views/users/_extensions.html.erb (L3)
In looking at forem/forem#16787, it felt like we would benefit from a
similar approach as we adopted for nullifying blank strings.
This refactor helps pave the way for a "normalize_text_for" method (or
some such thing; naming things is hard).
This change also involved introducing a `describe` block into the
associated spec.
* Fix broken documentation link
When sending an invitation, and smtp isn't configured, we show a link
to the old smtp-settings page, which has been renamed
email-server-settings in the admin documentation site.
* Remove explicit link target, assert link text is present
This spec failed when I changed the link target to match the
documentation. Rather than moving the target, I'm just asserting we
show a link to the docs (not where the docs are located).
* Only fetch RSS of users who can create articles
This commit leverages changes from forem/forem#16732 and
forem/forem#16763 to ensure that we're only fetching RSS feeds for
folks who have permission to create articles.
In addition, I chose to rename the variable `user_scope` to
`users_scope` to bring consistency in the variable names.
I also chose to refactor a private method so that it wasn't setting two
instance variables but was instead returning a value that could be used
to set an instance variable.
Closesforem/forem#16487
* Amending method name
Note, I'm not overly keen on writing permission tests for this
component. Why? Robust permissioning tests can create combinatorial
explosions. And there are presently no Rspec request specs for this.
So to add an automated test, we'd need to add a set of seed data that
seeds data that conforms to the emerging business logic of the policy.
And while this is easy with use case 1-1, it gets harder as we move
into more nuanced use cases. Instead we should rely on bombarding our
policy classes with lots of tests to let them demonstrate what we mean
when we say `if p.olicy(Article).create?`
Note, there is a potential relation to forem/forem#14807, namely if we
add a rich text editor to our comments, we may need to explore the
purpose and intention of this setting.
Closesforem/forem#16516
While in essence an idempotent change the purpose of this PR is to also
favor explicit method calls over callbacks. Note, we don't have an
associated model (e.g. Video) and instead rely on `:video` which Pundit
converts to VideoPolicy.
From my experience in rails it can become challenging to mentally parse
the various before/after action paired with the only and except. As
well as considering the timing of what happens when.
Further this commit helps provide a record of "work" towards an issue.
Closesforem/forem#16729
Reverses forem/forem#3806
This change makes it easier to resolveforem/forem#16487.
What do I mean by that? To address forem/forem#16487, I need to only
select user's who are authorized to create articles. In
forem/forem#16732 I added a method that will allow chaining of a User
scope. So with this current commit and forem/forem#16732, I'm
triangulating on an approach that will make that change easier.
I did not want to conflate those two, as mixing this PR's change and
what is necessary for the closing PR would create a more complicated
review. Not unduly complicated, but one that will require more tests
and a change in logic. And for someone reviewing the diff, those
concerns could easily be lost.
Yes, I have changed the method signature, but that method signature is
limited to one location:
```shell
❯ rg "Feeds::Import.call"
app/workers/feeds/import_articles_worker.rb
21: ::Feeds::Import.call(users_scope: users_scope, earlier_than: earlier_than)
```
So I look to the method signature a bit as an internal API, hence the change.
Related to forem/forem#16487
* Use correct table heading for username
* WIP add role filter and crayonsify forms
* styling
* responsive
* Add form back in that was removed by merge conflict fix
* Suggest only valid search options
* Add basic tests for search and filter by role
* Add aria for search field
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* Add email to aria
Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>
Co-authored-by: Paweł Ludwiczak <ludwiczakpawel@gmail.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>
As I'm looking at limiting feed fetching only for users who are able to
create articles, I needed a means of querying "Who are the users who can
create articles?" This bit of work was the smallest chunk that I could
think of.
These kinds of questsions are going to propogate (e.g. give me a list of
all the people who can comment? who can create articles in this space?)
For now, this pattern should suffice.
Yes, there's duplication of knowledge between the policy's instance
method and class method. And there's a bit of knowledge bleed between a
user's method (e.g. `user.any_admin?` and
`*Authorizer::RoleBasedQueries::ANY_ADMIN`) but that's a small "evil".
Related to #16486
* when there is no div for it, don't render CommentSubscription
* Retrigger CI
* Retrigger CI
* don't write an error to the comment div if its gone
This fixes an issue seen in tests, where the page transitioned while
we were running the articlePage async handler, so the article-body
element was gone (can't access dataset of null), and the error handler
catch block tried to access the innerHTML of another getElementById.
* Use existing found element `root` when setting the error message
No need to find a new comment-subscription div to write the error to,
we already named the one we're interested in at the beginning of the function
* Remove dots from chart when time range is Infinity
* refactor: 🐛 Remove dots using the chart options
* refactor: ♻️ Improve params of drawChart func
* refactor: ♻️ Use a variable showPoints
* only init mod tools if we're still on article page
* Update app/javascript/packs/articleModerationTools.js
Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>
Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>
