Prior to this commit, there existed duplicate logic between two of the
primary user feeds. This refactor introduces a new object which is
solely concerned with calculating scores to add to an article base score.
With this commit, we can introduce A/B testing by changing passing
different config values to `ArticleScoreCalculatorForUser.new`.
My suspicion is that for some of this, we might be able to better
leverage the database via select statements and SQL sums. However, that
is presently outside of what I'm prepared to tackle.
* Cleanup single reaction template
extract the title: link_to() target conditional outside the call to
translate
add missing closing paren for t()
remove comment (probably could stay if it's useful - explains why we
have a conditional at all)
* Fix indentation and ensure link has correct class
* Fix indentation and ensure links have a path
* Remove class from translate template
This was left from shuffling arguments to function calls
* Initial work to expand detection functionality
* Finish up functionality and tests
* Fix class name
* Fix tests
* Update spec/services/articles/enrich_image_attributes_spec.rb
Co-authored-by: Michael Kohl <citizen428@forem.com>
* Update spec/services/articles/enrich_image_attributes_spec.rb
Co-authored-by: Michael Kohl <citizen428@forem.com>
* Add one reaction to each article when seeding the database
* Add reaction to article when created
and sync reactions count.
There's still a gap where the redis-cached reaction count could be
present but not cleared during seeder runs (redis is disconnected for
caching and rails cache is set to the null store). Since we are able
to enqueue sidekiq jobs redis is live - but it's a bad idea to use
sidekiq's connection to flush keys in redis.
* Make the reaction count update script a no-op
* Remove unused data update script and limit user id queries
Pull all user ids into an array before you start creating articles,
and sample from the array rather than repeatedly asking for the first
user from a random ordered db result.
* fail if we can't create reaction
thanks @jgaskins
Co-authored-by: Jamie Gaskins <jamie@forem.com>
* Revert "fail if we can't create reaction"
This reverts commit c35f6d47d2e8956a240133b9e6ad0d144f1b4722.
There is a uniqueness constraint on (user, reactable, category) that
could be triggered when seeding due to random selection. It's better
to skip creation when a validation error occurs than to break the seed
completely.
There are expected to be 0-9 reactions per article, we don't require a
set number on each article, and likely won't notice any specific
problem if the random number is one lower than it would have been
because some reactions were skipped due to uniqueness violations.
Co-authored-by: Jamie Gaskins <jamie@forem.com>
* Adding guard clause for nill org feed score
There's no sense running a query if we don't have an organization. This
is a small refactor that might offer a most nominal speed bump
* Caching calculated value
Prior to this commit, we calculated the user_experience_level once per
article in the feed.
This change adds a cache, creating a minor computational improvement.
* 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.
* Try using Netlify pre-built yarn
* Add env vars to maybe trigger Yarn
* shrug
* Try using npm to build storybook
* Add npm install
* Update node version and use original command
* Update yarn version
* idk :(
* Try pulling versions from files instead of hardcoding
* Possibly in root already?
* never mind dynamic var usage is not supported
* yarn version prob unused b/c we technically use npm
* tag edit
* more
* chop chop
* spec
* Apply suggestions from code review
Co-authored-by: Michael Kohl <me@citizen428.net>
* french translation
* Fix I18n usage
* Apply suggestions from code review
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* lang
Co-authored-by: Michael Kohl <me@citizen428.net>
Co-authored-by: Michael Kohl <citizen428@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
For most of these this isn't a big deal because `class` is delimited
by whitespace, but it changes the meaning of the `id` attribute since it
must be an exact match (including whitespace). There is also a data-*
attribute or two in here that may have had their semantics changed.
The malformed link ended up messing up HTML further down the page,
hiding the "Unpublish Article" button because the browser didn't parse
it as a separate element.
As part of my reading and seeking to understand the
`Articles::Feed::LargeForemExperimental` class, I figured I would come
in and add parameters. In adding parameters, one thought I had was that
we can more readily create a data structure that allows us to tweak the
various implicit values.
This is a first step to help me understand and name the numbers.
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.
* feat: add a required attribute for the fields on Navigation Links that are required
* remove the weird margin
* we want the field to be required only when we don't have a vallue from form.object[:icon] that is in the craete view
* Update read path to match our current file layout
The files specified under "more files" were expecting to be
named *.{locale}.yml, like misc.en.yml or tags.fr.yml. However, our
project layout is using a directory prefix like misc/en.yml or
tags/fr.yml.
Consequently, the locale files for views were not being tested (a
previous update ensured that they were being loaded).
Change the load pattern to match the files we use in the app, and
ensure they're tested.
This introduces test failures which will be addressed in the next commit.
* Add back the original pattern
This matches files like devise.en.yml and other existing files in
config/locales (not already in a subdirectory).
* Update inconsistent interpolation
The english version had `{community}` while the french version had
`{name}` for the videos.heading key, checking the call site at
https://github.com/forem/forem/blob/main/app/views/videos/index.html.erb#L26
we have `community:`, passed for interpolation to the translate call,
so change the french translations to include that, rather than name
* Update missing pluralization for badges
* make travis happy
