**tl;dr** This PR is looking to provide a means to programmatically
assemble, from system configurations, what is currently the
[Articles::Feeds::WeightedQueryStrategy::SCORING_METHOD_CONFIGURATIONS][1].
Once we merge this PR, instead of having that constant, we'll initialize
the Articles::Feeds::WeightedQueryStrategy with the variant
configuration that we've assembled.
**Introduction**
This pull request has quite a bit going on, but as of now the production
code does not use any of it.
*Note: None of this code is bleeding into production code paths.*
Put your Ruby hat on and let's go for a ride. And apologizes for not
making this a smaller pull request. As I built this, I made many small
commits, but this became the smallest commit that provided the most
context without integrating into production code.
Before we get started, you may want to familiarize yourself with where
we're going. The `./app/models/articles/feeds/README.md` provides
further guidance; but fair warning reader, I have not reconciled the
README's language with what emerged as I wrote this pull request.
Those of you who are part of Forem Core team, you can read the
[Refinements to Our Present Feed Configuration][2]
to provide some insight into what's happening.
**Why not reconcile?** *Because I want to have all of the language and
thoughts available for you to help consider how best to name and model
this.*
**On to the Review**
The purpose of this pull request is to provide a mechanism for
engineering to provide a series of variant query levers. And to allow
us to easily configure those available query levers into a variant
query.
*Why the mix of Ruby configuration and JSON?*
The Ruby levers are meant to indicate that this is code we don't want to
expose in text based configuration because it might create SQL inject
points. (More on that when we integrate the code of this PR into the
production implementation)
The JSON levers are meant to indicate that "anyone" can pick the
appropriate/available levers and configure how they are set. In other
words, these values do not create SQL injection issues.
**What I Need from You**
I am putting this forward as a draft so you can read this code ahead of
time. I'm then going to schedule a synchronous code review where we
record our collective walk through of the implementation.
Closes:
- forem/forem#17268
- forem/forem#17269
- forem/forem#17270
- forem/forem#17271
- forem/forem#17273
Relates to:
- forem/forem#17308
- forem/forem#17245
[1]:6818ef3ed0/app/services/articles/feeds/weighted_query_strategy.rb (L84-L231)
[2]:https://forem.team/jeremy/refinements-to-our-present-feed-configuration-1p0c
From Anuj:
> Going forward we will be using heavy font-weight for the titles of all
> the admin sections. So it’s good consistency-wise.
This resolves that issue.
Closesforem/forem#17327
This div tag is subsumed by the line below (e.g. `<%=
application_policy_content_tag("div"...`). When the unclosed div is
present there is some undesirable layout offset. When it's removed,
things line up a bit nicer.
Discovered while attempting to reproduce forem/forem#17324
This commit modifies two things:
1. The sort order of the feed.
2. Locking in the next incumbent for the feed.
In consultation with Jamie regarding the SQL, I've switched from a WHERE
IN type clause to creating a JOIN. This allows for the calculated
relevancy score to become a sortable value for the feed.
Second the incumbent feed is a slight modification of the past winner;
based on the results of a long running experiment. This adjustment is
per conversations with product.
The past experiment ran since early February, so it's time to retire it,
regroup and move forward.
Closesforem/forem#17307
* add expandable search control at mobile size
* add invitations smaller layout
* replace missing member image
* tweaks following merge
* minor layout tweaks
* add actions to smaller view, update cypress specs
* ContextNotification model
* Create and destroy context notifications when notifications are sent/unsent
* Added a transaction when creating notifications and context notifications
* Documentation for context notifications
* Update article builder spec
In #16536 the tests were updated to match a change in the
logic (rather than "needs authorization" the response was "store
location", and we checked for those values).
In #17294 we re-adjusted the meaning of the second returned value from
Articles::Builder.call, and the call sites now label the second value
as "needs_authorization" again.
Restore the same name to the call sites in the spec, and simplify the
example descriptions (this might be a mistake).
* Restore long example descriptions
This restores the names of the test cases to the original.
There are two problems engrained in this:
1. The undocumented expectation of `/new` needed to other sites linking
with prefill information.
2. The prefill parameters being lost during the authentication process.
The first bug was introduced as part forem/forem#16536. The second is
likely introduced as part of a Rails upgrade (namely `request.path` most
likely once included the query parameters but with our current
implementation it does not).
Closesforem/forem#17292
Prior to this commit, we were looking at trusted user's reaction
counts. This is not the desired column to be testing.
See
f1d6291d00/app/models/reaction.rb (L25-L28)
```ruby
counter_culture :reactable,
column_name: proc { |model|
PUBLIC_CATEGORIES.include?(model.category) ? "public_reactions_count" : "reactions_count"
}
```
In the above code, all public reactions (hearts, unicorns, reading
list) go to public_reactions_count. Other things go to the
`reactions_count`; which is the thumbs up and thumbs down.
Fixesforem/forem#17260
* fix duplicate ID main nav
* fix duplicate ID other nav headings
* fix duplicate ID reading list count
* fix duplicate attribute class
* missing case readingCountList on feed
* missing dot in query selector
* Revert "fix duplicate ID other nav headings"
This reverts commit 4f495962c47e08f0f6c41e33e897b2db176432f5.
* use generic class 'other-nav' for tests
* pass a variable 'context' to the partial nav
* pass var 'context' to partial
* fix unwanted typo in partial sidebar
* Update app/views/admin/navigation_links/index.html.erb
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* Update app/assets/javascripts/initializers/initializeReadingListIcons.js
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* Update app/assets/javascripts/initializers/initializeReadingListIcons.js
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* js-reading-list-count renaming
* use data-testid instead of generic classes for tests
* fix typo in specs
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* Adding Feed Configuration documentation
This documentation outlines the scope and intention of the Feed
Configuration, it's primary purpose is to:
- Reference the historical context
- Establish a draft glossary
- Introduce high-level constraints
- Serve as a document to help with the task break-down
Closesforem/forem#17244
Relates to forem/forem#17245
Informs forem/forem-internal-eng#435
As a note, I'm looking at this document almost through the lens of
[literate programming][1]. In writing the document, I learned about and
attempted to describe the shape of this internal facing project.
Consider for a moment that we are not adding a new user facing feature
to the Forem code base. Instead we're scoping this project as helping
ease the experimentation process regarding the feed.
In other words the scope of this project is to help name and refine
different feed strategies to understand what each strategy means.
Eventually this could mean that creators and members can have a
future opportunity to select feed strategies that reflect the
aspirational community values of the feed (e.g. we want a feed that
favors comments, or neighborhoods, or the occassional serendipity).
[1]:https://en.wikipedia.org/wiki/Literate_programming
* Update config/feed/README.md
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* Update config/feed/README.md
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* Update config/feed/README.md
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* Update README.md
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
