* Bypassing validiation for data script
Prior to this commit, we attempted to run a script against all articles
using validation. In the case of DEV, we have lots of articles that
would no longer validate. This change now updates the column value
without running validation.
Relates to #16075
* Bump for travis
* Add boolean attribute main_image_from_frontmatter to indicate if cover image was set via frontmatter
* use article model spec for main_image_from_frontmatter test cases
* Add data migration script and spec for main_image_from_frontmatter for articles
* Update app/models/article.rb
Co-authored-by: Jeremy Friesen <jeremy.n.friesen@gmail.com>
Co-authored-by: Jeremy Friesen <jeremy.n.friesen@gmail.com>
Co-authored-by: Michael Kohl <me@citizen428.net>
Prior to this commit we had an inline logic check on whether or not to
render a navigation link. As we are looking to rollout the feature flag
for Listings, we needed to add another somewhat complex conditional.
This commit moves the inline conditionals to a helper function, which
makes testing the logic far easier. Especially since we need to bombard
the tests with the combination of 3 different boolean checks. (And one
of those boolean checks requires even more but could be stubbed).
There should be no UI differences, as we're presently assuming the
Listings feature is enabled.
Related to forem/rfcs#291
This failed in a main build today, testing shows this may give the
same domain very rarely
10_000.times.map do
Addressable::URI.parse(Faker::Internet.url).domain == Addressable::URI.parse(Faker::Internet.url).domain
end.tally
=> {false=>9998, true=>2}
Use a .dev domain (which is absent from faker's internet domain_suffix
list at
https://github.com/faker-ruby/faker/blob/master/lib/locales/en/internet.yml)
to ensure two unique domains are compared.
remove link to faker internals
* move home link to a customisable navlink
* add trailing slash for path
* update specs
* replace positions, set home link to -1
* add update script for adding home navigation link
* update data update script
* Removes FeatureFlag.enabled?(:creator_onboarding) from codebase
* Removes FeatureFlag.enabled?(:creator_onboarding) from specs
* Further cleanup, removal, and spec fixes
* Fixes the user_request_confirmation_spec.rb
* Reverts change to confirmation email button
* Revert revert after looking at designs again :(
* Removes redundant logo_png field from config + fixes test
* Rewords an expectation in user_uses_the_editor_spec.rb
* Revert removal of logo_png from Config images
* Removes CSS class from user_uses_the_editor_spec.rb
* Removes test from user_uses_the_editor_sepc.rb
* Removes unnecessary else from _logo.html.erb
* Adds back removed system spec
* Removes SVG-related code from _logo.html.erb
* Removes AsyncInfoController#use_creator_onboarding
* Fixes spec failues due to removed code
* Removes svg-related code (that I thought I removed already :/ )
* Re-removes FeatureFlag and logo_svg from _images.html.erb
* Remove newest instances of FeatureFlag(:creator_onboarding)
* remove instances where we use the creator_onboarding field from the base_data
* fix: redirect to the correct path in the reguistrations controller based on whether the user is a creator or not
Co-authored-by: Ridhwana <ridhwana.khan16@gmail.com>
Prior to this commit, in test, we had two different answers to "what is
the host of this application?" For Rails generates
URLs (e.g. `root_url`) we would get one answer. For our custom
`URL.url("/")` we got another answer.
This resolves and patches that up.
Closes#16347
* Base author reaction functionality
* Get logic more in place
* Finalize tests
* Add admin clause for new user points
* Add proper registered_at for seeded user
* Fix regsitered_at in e2e
* Add registered_at across the board in e2e tests
* Update comments
* Update spec/models/reaction_spec.rb
Co-authored-by: Jamie Gaskins <jamie@forem.com>
* Update spec/models/reaction_spec.rb
Co-authored-by: Jamie Gaskins <jamie@forem.com>
* Update spec/models/reaction_spec.rb
Co-authored-by: Jamie Gaskins <jamie@forem.com>
* Put points in constant and refactor points resave logic
Co-authored-by: Jamie Gaskins <jamie@forem.com>
The fundamental goal in this refactor is to make it easier to disable the
Listings feature set. By extracting a routes file (via the `draw` method [see
implementation details][1]), we can more easily disable those routes _en
masse_. This "crease in the code" could mean that we wouldn't need to update
the controller action logic. An update of that logic might mean going into
each controller action related to listing and adding a conditional for render
404 when listing is disabled.
Now, on to why this should work (not I did not open the application and click
around, but instead went with a "let the framework help me" mindset).
I believe that this should work; though there's a caveat†. Here's why I
believe this will work for extracting the listing routes:
Before making changes I ran the following:
```sh
$ rails routes | sort > original-routes.txt
```
Let's break that down:
- `rails routes` :: renders the routing table for Rails; this is how
Rails converts routes to the magic of stuff like `article_path(:id)`
- `| sort` :: to sort the results
- `> original-routes.txt` :: to write the results into a file.
Then I made the changes and ran the following:
```sh
$ rails routes | sort > updated-listing-routes.txt
```
And finally, I ran:
```sh
$ diff updated-routes.txt original-routes.txt
```
The results were as follows:
```sh
500c500
< api_organization_listings GET (/locale/:locale)/api/organizations/:organization_username/listings(.:format) api/v0/organizations#listings {:locale=>nil, :format=>"json", :to=>"organizations#listings"}
---
> api_organization_listings GET (/locale/:locale)/api/organizations/:organization_username/listings(.:format) api/v0/organizations#listings {:locale=>nil, :format=>"json"}
```
Note the only difference is that the above two lines, one has
`:to=>"organizations#listings"` and the other does not. However, the
`./spec/requests/api/v0/organizations_spec.rb` exercises the route; so I
assume a hiccup/foible in the route reporting.
There was no difference between the before and after files.
† Caveat: if for some reason the order of the routes mattered, this will
have messed that up. I don't think that is the case, but we shall see.
Related to forem/rfcs#291 and #16335
* Initial statement of intent regarding Listings
I want to use this pull request as a means of conveying intended
direction; namely that we want Forem administrators to be able to toggle
off (or on) the Listing feature set.
This refactor is a first, yet incomplete pass, that doesn't make the
code worse. Expect more of a similar vein as we work to put the Listing
feature set behind a feature flag.
Related https://github.com/forem/rfcs/issues/291
* Adding feature flag based on reviewer comment
Prior to this commit, we were treating all NavigationLink attributes as
unique. So, were we to change a position of one of the NavigationLinks
during the add_navigation_links rake task, we would have created a new
NavigationLink (and the only one would have remained).
With this commit, we're introducing the concept of the NavigationLink's
surrogate identity, that is to say if we have two NavigationLink objects
with the same `url` and `name` we should consider them the same
NavigationLink. This allows us to update properties of those
NavigationLinks (via the rake task) without the risk of creating new
entries.
This unblocks PR #16268 which addresses #16076.
* Added tag search to nav menu
* Added tag search
* Improved tags search results view
* Removed commented lines from the controller
* Fix specs for Search::Tag
* Prepare for tags search pagination
* Fixed Search::Tag specs
* styling
Co-authored-by: Paweł Ludwiczak <ludwiczakpawel@gmail.com>
* Convert symbol hash keys to strings when calling .perform_async
Fixes a warning from Sidekiq 6.4.0+ about perform_async arguments
which are not equal when passed to perform (`JSON.parse(JSON.dump(arg))`
should equal arg).
This is a safety measure to prevent passing objects (like classes, or
model instances) rather than their representations (like a class name,
or a model's attributes hash).
This warning will be an error in sidekiq 7
* Turn warning into an error in non-production environments
* Use string keys for reaction notification and article fetched
Missed these two on the first pass
* Update example argument hashes for #enqueues_on_correct_queue
Since this calls perform_async under the hood we need to pass json
safe hashes in the test cases as well.
https://github.com/forem/forem/blob/main/spec/workers/shared_examples/enqueues_on_correct_queue.rb
* Convert keys from FollowData#to_h to string before perform_async
I'm not sure enough where else (outside of notification) to_h is being
called, so I'm converting here when building args, rather than in
FollowData#to_h, which might be my next step.
* Let FollowData#to_h return a hash with string keys
Update spec to use string keys as well.
* Make to_h return string keys for ReactionData
Like FollowData, the #to_h method is only used to call
notifications (this is used to enqueue sidekiq jobs).
* Remove a key that was in the hash
Since reaction_data calls to_h, it gets string and not symbol,
keys. Call Hash#except with a key that was actually there.
* Refactoring cached_followed_tags
This refactor consolidates two queries and a loop into a single query.
One thing that is unclear is if we're ever really clearing the cache of
the user by id?
What follows is un-related to the refactor but appears to be related to #14937.
> First, I see we set a cache here:
>
> ebdaaaf15b/app/decorators/user_decorator.rb (L23-L34)
>
> But when I look at what busts the user cache, it doesn’t look like we’re busting it for the above cached location:
>
> ebdaaaf15b/app/services/edge_cache/bust_user.rb (L3-L22)
>
> What if anything am I missing in regards to cache busting?
* Adding some deprecated methods
* Updating documentation
* Bump for travis
* Bump for travis
* Updating comments to fix confusion
NOTE: I might be introducing more confusion, but at least
I'm addressing the prior confusion.
* Bump for travis
* Updating so we don't cache ActiveRecord objects
* Selecting only applicable attributes for caching
* Apply suggestions from code review
Co-authored-by: Michael Kohl <citizen428@forem.com>
* Adjusting code based on feedback
* Bump for travis
* Bump for travis
* Update app/decorators/user_decorator.rb
Co-authored-by: Michael Kohl <citizen428@forem.com>
Co-authored-by: Michael Kohl <citizen428@forem.com>
I saw the following error in Honeybadger:
```
[PROJECT_ROOT]/app/services/notifications/reactions/send.rb:61 :in `call`
old_json_data = notification.json_data
previous_siblings_size = notification.json_data["reaction"]["aggregated_siblings"].size if old_json_data
notification.json_data = json_data
[PROJECT_ROOT]/app/services/notifications/reactions/send.rb:20 :in `call`
[PROJECT_ROOT]/app/workers/notifications/new_reaction_worker.rb:16 :in `perform`
[PROJECT_ROOT]/app/models/notification.rb:84 :in `send_reaction_notification_without_delay`
[PROJECT_ROOT]/app/controllers/reactions_controller.rb:136 :in `destroy_reaction`
[PROJECT_ROOT]/app/controllers/reactions_controller.rb:168 :in `handle_existing_reaction`
[PROJECT_ROOT]/app/controllers/reactions_controller.rb:77 :in `create`
```
As I was exploring the error, I saw that we were making assumptions
about the data coercion. These are valid and somewhat stable
assumptions, but I wanted to look a little deeper into the situation.
This refactor helps consistently negotiate two situations where we're
transforming request-cycle models into lightweight data structures. It
also begins to show a path towards a generalizable "macro" for this behavior.
Related to #2122, #9534
* Don't set saw_onboarding automatically
This should be set by the onboarding checkbox update (user accepted
terms of service and code of conduct).
The issue is a PATCH request to track the last seen page is sent on
the first page, _before_ the user consents to the terms via the
checkboxes.
This issue is masked by a 422 unprocessable entity response when we
don't get a username in the patch request (next commit).
* Only validate username on the username page
Every other "last page seen" dialog was returning a 422 because the
username is only submitted in the 3rd dialog "build your profile".
Only validate the username field when it's submitted, process other
onboarding updates normally.
* Remove failing test
Since the onboarding_update action no longer sets saw
onboarding (since we send the patch before they've accepted the code
of conduct), don't test that we do.
There's a parallel test for the onboarding checkbox update later in
this file that covers the checkboxes have been accepted.
* overwrite instead of merging user_params
We now initialize it empty, no need to merge params here.
On the DEV.to database, I ran the following SQL:
```sql
SELECT name, short_summary FROM tags WHERE short_summary LIKE '%<%';
```
There were three tags with a `<` in them:
- changelog
- cnc2018
- javascript
This PR will tidy up our short summary as part of saving a tag. It will
also tidy up the existing tags.
* Let override of ToS and CoC show in onboarding
Prior to this commit any changes to the terms or code of conduct were
not reflected in the onboarding links.
With this commit, I'm leveraging a newly created method that first
checks if there's a page for the given slug. If there is, use that
page's copy.
To test:
- Overwrite default /terms page on a Forem
- Navigate to /onboarding or sign up as a new user
- Click on Terms of Use link text
- Review Terms of Use
Closes#15296
* Adding spec around not passing a block
* Removes the Getting Started section and related code from the config
* Removes the admin_manages_configuration_spec.rb
* Removes "Required" tags from Config
* Reverts removal of activateMissingKeysModal
* Removes mandatory.rb
