* Adding some docs for ApplicationHelper
This is the happy byproduct of my waiting for Heroku's git server to
come back online.
What follows is my attempt to provide inline documentation for
application helper methods. I haven't done all of them, but this is my
effort to inch forward the state of our internal documents.
Along the way, I've discovered a discrepency (and noted it but am not
going to proceed with any refactor).
* Update app/helpers/application_helper.rb
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
In helping track down forem/forem#17041 I was looking at our cache
busting logic. We were looking up a constant via the `.const_get` call
from a string already defined inline. This refactor short-circuits
naming a string, capitalizing it, then looking in the class's registered
constants.
There's also a subtle bug in the original; When `provider` equals
"another_cache", the `#capitalize` method would return
`"Another_cache"`, whereas `#classify` will return `"AnotherCache"`.
Below are some benchmarks for the change.
```ruby
require "benchmark"
module EdgeCache
class Bust
def by_class
Fastly
end
def by_const_get
self.class.const_get("fastly".classify)
end
end
end
Benchmark.bmbm do |x|
x.report("by_class") do
1000.times do
EdgeCache::Bust.new.by_class
end
end
x.report("by_const_get") do
1000.times do
EdgeCache::Bust.new.by_const_get
end
end
end
```
```shell
> bin/rails runner /Users/jfriesen/git/forem/bench.rb
Rehearsal ------------------------------------------------
by_class 0.001239 0.000186 0.001425 ( 0.001342)
by_const_get 0.011398 0.000136 0.011534 ( 0.011538)
--------------------------------------- total: 0.012959sec
user system total real
by_class 0.000973 0.000030 0.001003 ( 0.000969)
by_const_get 0.011102 0.000032 0.011134 ( 0.011104)
```
* Trigger Build
* WIP - use turbo to render html for create post button
todos and discussion topics
[] add tests (probably gonna be a lot of cypress 😅😭)
[] nominclature & file structure for authorization controller/routes
[] is this "authorizations" namespace approach the best?
[] authorization strategy relies on current user and caching doesn't
like `current_user`
* Use URL helper and not change the policy just yet
* put the turbo frame behind a feature flag
* Trigger Build
* Use URL helper and not change the policy just yet
* :)
* Add some test coverage and use same feature flag for consistency
* rubocop
* more rubocop
* trigger build
* adding a small sleep to see if specs pass 😩
* remove sleep
* disable Turbo session drive
* WIP
* Revert "WIP"
This reverts commit ce838672069fd703122156a0d82e5e6ad5398e0a.
* Moving spec from dashboard to root
With forem/forem#16913 the underlying test broke because we were
redirecting on the dashboard.
Co-authored-by: Jeremy Friesen <jeremy.n.friesen@gmail.com>
* initial work to show the expand/collapse search and filter on member index
* replace icon with actual button
* simplify logic in pack
* add cypress tests
* make sure both fields part of same form
* add an indicator in mobile view when field has a value
* make sure empty params treated same as missing params
* make sure indicators stay in step with current user input
* partials for inputs
* rename pack file
* Add toggle button to 'New Forem Secret' form
* Display messages using i18n
* Fix from snake case to kebab case
* Fix a bug when the specified class did not exist
* Avoid using robohash for article cover images
If their service slows down, loading the cover image times out in
system tests.
Use a locally served image file instead.
* Specify expected image format for view object test
We were relying on the internals of Faker to return a robohash.org url
and checking that we included that in the cloudinary url.
Rather than relying on the default behavior, explicitly pass a
robohash url in the main image for this spec.
This is failing for me (and had been flaky previously).
https://app.travis-ci.com/forem/forem/builds/248455560 blocked a
deployment because of an error (`@followsRequest` timed out waiting).
I'm able to reproduce this locally and disabling until we can find a
stable fix.
Prior to this commit the following situation existed:
> The path /dashboard/analytics/org/:id requires user
> authentication (e.g. signed in). However, it does not enforce
> authorization. Anyone can see this page. The page, however, uses
> javascript to populate the data. So no information, aside from the org
> name associated with the :id leaks out. The javascript API end point
> enforces organization membership.
>
> I would expect that the authorization in the HTML rendering would be
> the same as the javascript API end point.
This commit ensures that the dashboards#analytics end point uses the
same policy logic as the API analytics end points. Further, it keeps
folks who aren't org members out of the base HTML page for other orgs.
Closes forem/forem/#16985
There are two existing listeners for the `Audit::Logger`: `:moderator`
and `:internal`. (Note: during tests we ignore the :moderator and
:internal logs as defined in [config/initializers/audit_events.rb][1].)
Using `rg "Audit::Logger\.log\(:internal," --files-with-matches`, the
`:internal` listener is found in:
- app/controllers/admin/secrets_controller.rb
- app/controllers/admin/settings/base_controller.rb
- app/controllers/admin/settings/general_settings_controller.rb
Using `rg "Audit::Logger\.log\(:moderator," --files-with-matches`, the
`:moderator` listener is used in:
- app/controllers/rating_votes_controller.rb
- app/controllers/comments_controller.rb
- app/controllers/stories/pinned_articles_controller.rb
- app/controllers/admin/response_templates_controller.rb
- app/controllers/admin/tags_controller.rb
- app/controllers/admin/articles_controller.rb
- app/controllers/admin/users_controller.rb
- app/controllers/admin/reactions_controller.rb
- app/controllers/admin/tags/moderators_controller.rb
- app/controllers/tag_adjustments_controller.rb
- app/controllers/reactions_controller.rb
The `admin/spaces#update` action is most similar to the `admin#settings`
actions, which is why I chose `:internal`. I am looking for further
guidance on documenting this little area of the application (in
particular providing a data dictionary of :internal and :moderator).
Closesforem/forem#16957
[1]:https://github.com/forem/forem/blob/main/config/initializers/audit_events.rb#L9-L11
This commit provides two things:
1. Some notes related to my analysis regarding the dashboard
2. Conditional redirects and rendering based on article policies
The code comments say most of what I want to say, but to reiterate:
When a user can't create articles nor do they already have published
articles, then we don't want to avoid showing them stats related to
articles.
Closesforem/forem#16913
Related to forem/forem#16908 and forem/forem#16931
* improvements to profile preview card cypress spec
* woops - missed staging files
* make sure follow buttons have been initialized
* wait on follow button fetching on load
* pagination theme for new admin view
* align pagination widget correctly in desktop view
* remove bottom pagination
* re-add pagination to the bottom
* use block styling for the links
We already have a partial unique index for this column scoped on
`published = true`, which is still useful. This index does not make that
index redundant because that index is used to enforce a constraint that
we *only* want to apply to published articles. This index will be used
when `WHERE published` is not part of the query.
* Add a comment, and a safe default value for cloudinary
When moving a site from imgproxy to cloudinary, we observed that
wrapping the cloud name in quotes caused off-looking urls (with the
user name in %22 escaped quotes).
Additionally, if cloudinary is enabled, cloudinary secure should
be set to true. We leave the others blank to prevent conditionally enabling this
service (we check for ENV var presence) mistakenly, but the secure
flag won't turn it on or off and is safe to keep a default value.
* remove unnecessary comment
quoting the env vars had no effect when tested.
As part of my AuthN/AuthZ work I'm reviewing policies. I've been
looking at Dashboard pseudo-policies. The dashboard has some implicit
organization policies that I'm looking to expose and describe.
This refactor simply leverages methods already on the user.
Discovered while working on forem/forem#16985
We only have one reference to the UnauthorizedError, which is shadows
the ApplicationPolicy::NotAuthorizedError. This commit removes the
exception.
Related to forem/forem#16985 but only barely
* Removing a JS message about connect
Related to forem/forem#14734
* Remvoing another reference
I ran `rg Connect[^i]` to see about any remaining references to
Connect. This one seems to be the last.
* Adjusting article copy to be more general
Our language regarding articles needs minor revisions to speak a bit
more generally about content. This follows on the features of
AuthN/AuthZ work to allow forem admins to configure their forems such
that a subset of their forem members may not have the ability to create
articles.
Closesforem/forem#16890Closesforem/forem#16891
* Update app/views/users/_notifications.html.erb
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
As I'm looking at the dashboard, there's lots of small duplication.
This refactor is a way to help me collect my thoughts regarding how to
approach the larger issue at hand.