* WIP: add a creatore settings form
* WIP: updat the controller to use the Creator Settings FOREM
* feat: use the creator settings form for the new action
* feat: add some default values for the new action
* a note about form data
* update the initiaize function to set some default values
* feat: update the form to use the model data
* feat: permit adn use the attributes within creator_settings_form
* update the flash error
* refactor: require and permit parameters
* chore: use booleans, set defaults and validate the form
* spec: update all the creator_settings tests
* chore: remove comment
* refactor: use self
* feat: aggregate failures'
* chore: remove the logo uploader in the controller
* refactor: update error handling
* feat: update the wasy the controller handles success and error
* chore: remove the resource errors
* feat: show flash message on new line
* fix: use a redirect so that we can get back to /new
* refactor: pass these values through as they seem to be caching whne setting them as default
* chore: change default values
* spec: update tests
* Fix CreatorSettingsForm specs
* fix: use a boolean for public
* spec: add another test for the success var
* fix: radio button labels to correspond + cyress specs
* spec: update based on new changes
* spec: update the params and the expected output
* spec: update the comments and status
* feat: no need for the initialize as we use Active Record Attributes
* feat: update the tac and coc to be persisted when ticked
* fix: amend spec
* blank space
* Message
Co-authored-by: Michael Kohl <me@citizen428.net>
* User decorator (and spec) should use `trusted?`
Fixes a few issues seen in an rspec run that show as:
DEPRECATION WARNING: User#trusted is deprecated, favor
User#trusted? (called from config_body_class at
/opt/apps/forem/app/decorators/user_decorator.rb:58)
And here:
/opt/apps/forem/spec/decorators/user_decorator_spec.rb:112
/opt/apps/forem/spec/decorators/user_decorator_spec.rb:121
/opt/apps/forem/spec/decorators/user_decorator_spec.rb:130
/opt/apps/forem/spec/decorators/user_decorator_spec.rb:139
* prefer User trusted? to trusted
DEPRECATION WARNING: User#trusted is deprecated, favor
User#trusted? (called from permissions at
/opt/apps/forem/app/models/rating_vote.rb:25)
* Prefer trusted? to trusted in user spec
* Use warned? rather than warned in admin article partial
* use trusted? rather than trusted in moderator requests spec
* Prefer trusted? to trusted in moderations controller
* Prefer trusted? to trusted in moderations view
* User auditable? should call trusted? and not trusted
Deprecations go rolling right along.
* Invert guard clause logic to be positive
The original "return unless multiple negated conditions hold" guard
was cumbersome.
Invert to return if any of the exceptions apply, namely:
- this is a comment or readinglist rating (rather than explicit),
allowed for all
- this rating is from a moderator/trusted user (allowed)
- this rating is offered by the article's author (allowed)
I had intended to also remove the safe navigation operators (since it
wasn't clear why there would be a null user or null article, as
rating_vote joins users to articles with a score), but the builtin
validation tests (is expected to validate ...) build objects with
missing attributes, and raise errors when the spec is run.
`User#trusted?` is more Ruby idiomatic than `User#trusted`. This helps
align the various "User#question?" methods.
I've added a deprecation warning, but don't believe that this is active
in the code-base.
* wip - Got logo upload working
* Now have the logo rendering in the header and in the admin image config section.
* Small layout tweak for admin -> config -> images -> logo.
* feat: create a logo uploader with some tests
* feat: use the logoUploader instead of the ArticleImageUploader
* feat: return early because svg's do not contain exif or gps data
* chore: we can move the raise outside the transaction as the rest of the transaction won't execute if we raise an error
* feat: add a size range
* WIP: resize an image to a random number for now
* hid the logo behind a feature flag and kept logo_svg as is in the site header.
* Added the jpe file type to the logo uploader.
* Skipped the resizing of an image if it's an SVG in the logo uploader.
* Added content types to the content type logo uploader allow list.
* Synced logo validation with frontend and backend.
* Removed unnecessary ALLOWED_PARAMS elements.
* feat: update the logo upoader and tests
* chore: remove comments
* chore: remove comments
* feat: update the resizing for the images + add the correct content type
* spec: test the versions
* fix: update the Constant
* feat: add the versions of the logo
* feat: populate the settings correctly and consistently
* feat: add an random string to the file name to avoid caching issues
* feat: amend the logo layout
* chore: remove comments
* spec: update
* feat: image type whitelist
* feat: update the logo css and also just use resized_logo and remove mobile resize
* feat: add a max-height
* only add site-logo if the feature flag is off
* Renamed IMAGE_TYPE_WHITELIST to IMAGE_TYPE_ALLOWLIST
* Update app/controllers/admin/creator_settings_controller.rb
Co-authored-by: Michael Kohl <citizen428@forem.com>
* Update app/uploaders/logo_uploader.rb
Co-authored-by: Michael Kohl <citizen428@forem.com>
* Update app/uploaders/logo_uploader.rb
Co-authored-by: Michael Kohl <citizen428@forem.com>
* Update app/uploaders/logo_uploader.rb
Co-authored-by: Michael Kohl <citizen428@forem.com>
* Update spec/uploaders/logo_uploader_spec.rb
Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>
* Update spec/uploaders/logo_uploader_spec.rb
Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>
* chore: revert admin change
* refactor: use a static value for directory
* feat: freeze constants
* feat: remove the logo requirement
* chore: spacing
* remove logo requirement
Co-authored-by: Ridhwana <ridhwana.khan16@gmail.com>
Co-authored-by: Michael Kohl <citizen428@forem.com>
Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>
Given that we have the roles of `:tech_admin`, `:admin`, and
`:super_admin`, I don't want the surprise of assuming that `user.admin?`
means that they have the role of `:admin`.
* Fixing weighted query hotness grab logic
Prior to this commit, I carried over (albeit imprecisely) logic from the
LargeForemExperimental. That logic was to help limit articles to those
that were published since the user's latest page view.
However, I introduced a bug in this transcription. Now both
LargeForemExperimental and WeightedQueryStrategy use the same logic to
determine the oldest publication date to search for in the feed.
This resolves a bug reported where users were not seeing a large number
of items in their feed.
Incidentally, if a forem has little activity in the 18 hours, there
might be very few items in the feed.
I believe, going forward, we may need to better parameterize how many
hours is considered "stale since last page view".
Related to #15240
* Fixing implementation detail
* Extracting helper method
Prior to this commit, I had introduced a method an put it in a less
ideal module space. This commit extracts that method to a more readily
shareable module space.
I've added a few more specs to help clarify and verify behavior.
* Updating documentation
* Renaming and documenting variables/constants
* Fixing that which I broke
* Adding tags to data tracing
Prior to this commit, the feed's class was part of the resource. That
resulted in a more opaque view of things.
With this change, we're adding tags to the view so we should have a
better sense of filtering.
* Bump for license/cla test
* AB Test for Feed Strategy
Adding the plumbing for performing AB Tests on the feed strategy.
You can add the environment variable `AB_TEST_SERVICE_FEED_STRATEGY` to
specify the field test to run.
For example, from the forem application home directory you can run the
following to force a strategy for all users:
```console
AB_TEST_SERVICE_FEED_STRATEGY=original bin/start
```
Forcing the strategy can significantly help with quality assurance testing.
* Adding configurable weighted feed strategy
I wrote about this exploration on [DEV.to][1]. This particular
implementation provides significant configuration options for A/B
testing.
At it's core this `Articles::Feeds::WeightedQueryStrategy` provides a
means to query articles that are relevant to the user. The
configuration helps us test and refine those values.
Further more this implementation introduces the idea of some common
interfaces for feed strategies. I'm not happy with the naming
convention but we can work to change that.
This even hints at a mechanism for administrators to configure.
Related to forem/rfcs#314, as I work towards understanding the feed
algorithm. And #15180
[1]:https://dev.to/jeremyf/practicing-postgresql-and-postulating-improvements-5m
* Renaming AbTestService to AbExperiment
I'm loath to end any production class's filename with "test". I don't
want minitest or rspec coming along and attempting to "test" that file.
In addition, I find that Experiment better describes the concept.
Also, per conversations with Michael K, we favor nouns objects to go
in app/models and verb objects to go in app/services. That's a great
separation.
* Factoring AbExperiment into a Controller concern
I wish this could be a stand-alone module but the implementation of
field test has cases that require controller context.
* Sanitizing configurable weighted strategy cases
* Minor refactoring of coding structure
* Renaming parameter for clarity
* Renaming variable to avoid confusion
Prior to the commit, `env` is a controller variable. The injected `env`
variable didn't collide, but did create just a bit of confusion.
With this change, I'm trying to disentangle from that variable.
* Factoring AbExperiment away from mixin concern
Per discussion with Michael K, we both agree that we do not like
controller concern mixins. They can quickly complicate the legibility
of any given codebase.
This refactor provides a common and simplified interface for a Plain Old
Ruby Object (PORO) that accounts for the implementation considerations
of the underlying "field_test" controller method.
* Adding Datadog.tracer.trace around feed actions
* Adding featured_article_factor
Prior to this commit, there was no consideration for how moderators
engage with an article. This is a first nudge towards that effort.
* Removing spec
Add a spec that says yes or no we're feature testing something seems
fragile. Namely, why are we wanting to assert that as a truism.
* Helping ensure we're "tracing" the same things
From the code comment:
> Hey, why the to_a you say? Because the
> LargeForemExperimental has already done this. But the
> weighted strategy has not. I also don't want to alter the
> weighted query implementation as it returns a lovely
> ActiveRecord::Relation. So this is a concession.
* Addressing parameters for tracing function
* For weight feed query only count positive tags
A user assigns explicit points to the tags that they do or do not want
to see. This change treats the negative explicit_points as an indicator
that they don't want to see the tag.
* Adding weighted factor for privileged user reactions
* Updating comments to better clarify
* Adding additional constraint for published_at
* Updating documentation and adding logger
* Adding documentation concerning method call
* Upping logging level for experiments
See conversations in https://github.com/forem/forem/pull/15240
* Adding group by clauses to appease postgres configs
* Injecting relevant scoring method's joins
Prior to this commit, we included all of the joins regardless of the
scoring methods chosen. With this commit, we only inject joins that are
used with the commit.
What this would allow us to do is to then have some pre-cooked scoring
methods but not turn them on. Or to turn off the more expensive scoring
methods.
* Adding feature flag for feed strategy experiment
* window.ForemMobile namespaced functions
* Fix broken Buildkite
* Wait for data-loaded before using Runtime in pack
* Bring back sprockets base
* Better error handling
* Smal fixes to ConsumerApp to support Android platform
* Fix typo
* utilities/waitForDataLoaded.js
* Update app/javascript/utilities/waitForDataLoaded.js
Co-authored-by: Nick Taylor <nick@forem.com>
* Review feedback
* Fix failing spec
* Cleanup promise
* Remove old utility file
* Add Cypress check for namespaced availability
* More specs
* Refactor to rely on ForemMobile for native bridge messaging
* Fix spec/requests/api/v0/instances_spec.rb
* Fix devices spec
* Remove changes to /api/instance
* Refactor to dynamic import
* Fix typo
* Fixed custom event detail payload for tests.
* mock ForemMobile function instead of webkit call directly
* failing jest test debug
* Another attempt
* Fixed broken test that was missing an onMainImageUrlChange prop on the component.
* Fixed mobile bridging E2E tests.
* Move Cypress tests to mobileFlows
* Add JSDocs + cypress spec for injectJSMessage when logged out
* Cleanup + Disable native image upload until AppStore approval
Co-authored-by: Nick Taylor <nick@forem.com>
Co-authored-by: Nick Taylor <nick@dev.to>
* WIP: Add COC and TOS checkboxes to Creator Settings form
* Adds assertions to the creatorSettings.spec.js E2E test
* Removes comments and unnecessary code from Admin::CreatorSettingsController
* Removes params from transaction
* Updates creatorSettings.spec.js to fix checkbox-related failures
* Adds a note to the COC and TOS checkboxes
* Guarding against spam from OAuth Sources
Prior to this commit, when an administrator had indicated blocked email
domains, those blocks were not applied to identities created via the
OAuth sources (e.g., Twitter, Facebook, etc). With this change, we're
hooking into the similar logic flow as suspended email accounts.
Related to #15403, #15397, and forem/rfcs#281
* Adding class documentation to exception
* Update app/services/authentication/authenticator.rb
Co-authored-by: Ridhwana <Ridhwana.Khan16@gmail.com>
Co-authored-by: Ridhwana <Ridhwana.Khan16@gmail.com>
* Remove Connect
* Remove more Connect specs
* Remove a lot more Connect code
* 🚮
* It all has to go
* Explicitly add httpclient
* Update application layout
* Remove messages association from User
* Start fixing specs
* reintroduce util function and refactor references
* Remove Connect Cypress test
* Fix more specs
* Remove Connect from listings
* Ignore contact_via_connect column on listings
* Remove contact_via_connect usages
* Ignore mod_chat_channel_id on tags
* Drop Connect tables
* Remove email_connect_messages from user notification settings
* Re-add httpclient 2.8.3
This was mistakenly removed as a merge conflict
* Don't need to exclude removed chat channel file
* Remove unneeded style for chat channels
* Remove unneeded channel list prop type
* Remove chat channels index/connect-link from getPageEntries
* Re-add comment from httpclient in Gemfile
* Remove connect references from mailers
Tag Moderators no longer have a chat channel
No longer will users be notified about new messages (there won't be
any)
No longer will users be notified about channel invites (you can't
invite anyone anymore)
* Don't configure Pusher and remove PUSHER_* from .env_sample
since it's removed from gemfile, the Pusher constant will not resolve, if this is
configured in the environment variables we'll fail to boot.
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Dan Uber <dan@forem.com>
Prior to this commit, we had filtering options for email registration:
1. Specific allowed domains.
2. Implicitly allow all domains if no allowed domains specified.
With this commit, we add another option: The adminstrator may block one
or more domains from registering.
Closesforem/rfcs#281
Don't allow passing `tag[name]=` in post params to admin tag update.
There's no field for this in the view, it's not expected that someone
change the name (it serves as a natural key) - it's preferrable to
alias an old name to a new tag if a renaming is desired.
* Add a failing test for comments index of deleted article
After https://github.com/forem/forem/pull/15052 removed the (also
broken) deleted commentable template and the redirect, requests for
comments from deleted articles raise no method errors (initially
trying to set the page title to "Comments for commentable.title" but
the assumption that @commentable is non-nil is present in several
other places.
This test currently fails (by design). Either we want to update the
controller so that comments from deleted articles are no longer
viewable (returning not found, as we did prior to
https://github.com/forem/forem/pull/5199 or making the view safe for
the case where @commentable is nil and no @root_comment is present.
These requests are happening quite frequently (@maestromac and I
suspect the sitemap may contain these pages and crawlers are visiting
the site from a published link), as evidenced by
https://app.honeybadger.io/projects/66984/faults/81994265
* Add request spec for deleted article scenario
Additionally, relabel the legacy spec (updated during #15052) to
clarify we do not render the deleted_commentable_comment view (this is
testing the comment.path, rather than the article.path/comments index
The scenario with comment.path sets `@root_comment` in the controller,
so does not trigger errors on the `@commentable` method calls.
* Update view
pass 1: get the errors to stop (need to check the rendered page is
also usable, the comment tree is not shown when commentable is nil and
this might be a huge usability/correctness issue).
* extract logic from comments index to methods and update tests
We no longer expect deleted article's comments index to render (should
return 404), only direct links to comments of deleted articles.
Only assign @article in the index for the view if it is in fact an
article (not a podcast episode, it's possible `@root_comment` was for
a podcast episode).
* remove unneeded (and soon to be incorrect spec)
No longer want or need to test for the case where root comment is nil
and so is commentable. This was an exploratory spec (scaffolding) and
can be removed.
* Undo nil safety changes to comments index
and fix typo in comments spec
* Adds a help icon to the creator settings setup page
* Moves the help icon styles from the a tag to the inline_svg_tag
* Removes reused class and aligns help icon to the right
* Refactors help icon out into a partial used across necessary views
* Moves .admin-help-button class from admin.scss to scaffolds.scss
* Adds further utility classes to align help icon as expected
* fix: closing divs and alignments
* fix: closing divs, alignments and add z-elevate
* refactor: remove the confirm email css annd add it to setup_mode
* improve mobile responsiveness
* feat: redirect directly to the new_admin_creator_setting_path from the confirmation email adn the signup
* feat: update the test to no longer show the referral which seems no longer relevant
* fix: update the tests to conform to the new URLS
* update the link
* Caching sum privileged reaction scores
This relates to work on improving the feed. Namely that in the current
proposal in PR #15240 I'm not accounting for how privileged users have
given feedback on an article.
With this change, I will now have a cached value on the articles table
that can be a proxy for how the privileged users have reacted.
In addition there's a small refactor that moves a constant to the
correct scoping object.
Dependent on #15283.
* Fixing method name to `exists?`
Prior to this commit, I was using `exist?` which doesn't work for
ActiveRecord::Relation objects.
* Downcase `followable_type` to fix wrong follows creation bug
* Use `follow_button` helper
* Remove puts statement
* Revert use of follow_button helper
* Remove downcase of followable_type from helper
* Refactor switch case for better consistency
* Add e2e test for follow organization from article page functionality
* Split cypress `it` block into two
* Add additional test to following organizations from dashboard
* Remove downcasing of followable_type for decorated objects
* Fix failing follow back spec
* Force capitalize `followable_type` param
Co-authored-by: Mac Siri <krairit.siri@gmail.com>
* Update app/controllers/follows_controller.rb
Co-authored-by: Mac Siri <krairit.siri@gmail.com>
This is a hypothetical and experimental interface change. The goal of
the change is to begin looking at what a "Feed"'s method interface would
look like.
I would like to reduce the controller logic necessary for deciding on
different strategies, and instead defer to a builder/strategy lookup
pattern.
* Change display from "created by user" to "reacted to user"
The reaction user_id is the creator (who reacted) and not the
target (reacting to what). On the user edit page, show reactions to
the user, rather than reaction from the user.
* Removing state change and unused method
Prior to this commit, the `@comment_weight` value would change. This is
not ideal as depending on the call sequence, can notably change the
output. I suspect this state change occurred so as to not alter an
underlying spec.
What this change does is remove the state change, removes a dead method,
renames a method (to the dead method name), and leverages
parameterization to better test a spec that was brittle based on
possible state changes.
I believe, from a logical stand point, that this change does not impact
the functionality nor the actual logic that is part of the production
call path.
* Marking methods as @api private
The goal is to highlight that we really shouldn't be calling these
outside of their contained class. Ideally, I'd love to make them
private methods, but there are specs and would prefer to not use
`__send__` to change those specs.
This is a noop change.
* Renaming method to refelct returned param order
Prior to this commit the "default_home_feed_and_featured_story" returned
an array of `[featured_story, default_home_feed]`. The method name and
the order of the returned values were misaligned. This change helps
align the method name and the order of those returned values.
Note, methods such as `each_with_index` have an `element, index`
parameter order.
This is the counter-argument to #15152 - filter in the controller
rather than adding lots of conditional logic to the view to handle the
case where there was a vomit reaction left for a deleted user causing
the moderation reports page to fail to load.
* Fix and clean up podcast pages
* Adjust tests
* Fix a test
* Fix missing i18n
* Fix tests
* Fix tests
* Fiddle with test
* Sure up css and tests
* Add featured as allowed param
* Fix a couple tests
* xit out test
* Update app/views/podcast_episodes/index.html.erb
* Update app/views/podcast_episodes/index.html.erb
Co-authored-by: ludwiczakpawel <ludwiczakpawel@gmail.com>
Co-authored-by: ludwiczakpawel <ludwiczakpawel@gmail.com>
* When a user is blocked from commenting on an article, show correct error message
* Created new error type ModerationUnauthorizedError and changed error text for it
* Move hard-coded Tags index copy to [en,fr] locale files
* Use tags.pluralize for a better page title implementation
* Add better french translations
* Once more fr translation fix
* Remove .pluralize from title i18n
* tokenize settings menu and add French translations
* add quotes around tokens
* add tokens in alphabetical order
Co-authored-by: Ben Halpern <bendhalpern@gmail.com>
