This is a "quick and dirty" hack. Do I like it? No. Do I want to delve
into further debugging/refactoring of [a 72 line method][1]? Not at the
moment. There are a few pathways forward, but for now, this is the
pathway to hopefully give us insight into the
**Why will this likely address the issue?**
Prior to this commit, for an experiment we would render each of the
goals (anywhere from 8 to 11); each of which would require 2 expensive
queries; which means about 16 expensive queries.
After this commit, the experiment page has none of those expensive
queries; instead each goal now runs the 2 expensive queries.
The hope is that this will help us show the results (albeit on multiple
pages) while coming in under the response time out handlers we have in
place.
**Why no tests?**
Ugh; I know right?!? I'm beginning to ask that myself. But for now,
because this is only visible to tech_admins, the consequences of
breaking are limited. In otherwords, this is not a customer facing
feature, so it can be a bit less robust in it's testing. At least
that's the rationalization I'm establishing.
Further, local tests would not reveal the production environment
complications of large data sets. The aforementioned expensive queries
are blisteringly fast on my local machine...in part because I don't much
field_test experiment data.
Closesforem/forem#17981
Related to:
- forem/forem#17895
- forem/forem#17869
[1]:ab2d7d29d0/lib/field_test/experiment.rb (L98-L160)
Prior to this change, we setup our feed variant query to have a cached
randomization seed. For a given user this seed is cached for 15
minutes. The goal of this is to provide mostly consistent sort orders
on the feed articles during that 15 minute window.
With this change, we allow for a feed variant to say ignore the cached
seed and generate a new one for this call.
I have also added explicit settings to each of the feed variants to
reflect our desired intentions for those features.
Relates to
- forem/forem#17833
- forem/forem#17826Closesforem/forem#17940
This change introduces several things things:
1. Fixes the performance of the `/admin/abtests` page; instead of
rendering all of the experiment results, just render the overview.
2. Adjust the show page for an experiment to show the summary of
results.
3. Favor the existing "started_at" and "ended_at" attributes of
FieldTest (see https://github.com/ankane/field_test#config)
4. Update the field test experiments to include the `started_at` and
`ended_at` attributes.
5. Skip processing all non-active experiments in our conversion handler.
Why are there no tests? This is a page that is Admin only and is only
visible for reporting purposes. So, it's not quite worth writing tests
as you'd need lots of data. So we'll assume the logic from the upstream
[field_test gem][1] is adequate.
Closesforem/forem#17869
[1]:https://github.com/ankane/field_test
This commit contains two separate but related changes:
1. A check-list for creating a new experiment.
2. A new experiment along with declaring a winner for the previous experiment.
Below are the diffs of the two new variants versus the incumbent for the
experiment:
```
❯ diff config/feed-variants/20220603-variant-b.json config/feed-variants/20220518-variant.json
3,4c3
< "description": "As 202205518-variant but with modificiation to `order_by` lever.",
< "order_by": "final_order_by_random_weighted_to_score",
---
> "order_by": "relevancy_score_and_publication_date",
```
```
❯ diff config/feed-variants/20220603-variant-a.json config/feed-variants/20220518-variant.json
3d2
< "description": "As 202205518-variant but with modificiation to `matching_positive_tags_intersection_count`.",
69c68
< [0, 0.5],
---
> [0, 0.7],
```
Closesforem/forem#17822
Prior to this commit, as a matter of undocumented practice, we would
overwrite past experiments with upcoming experiments.
With this commit, we're leveraging a feature of the field_test gem:
declaring a winner. Instead of overwriting an experiment with the next
experiment, we're going to declare the expiring experiment's winner then
prepend to the ./config/field_test.yml the new experiment.
The [field_test/app/views/field_test/experiments/index.html.erb][1] then
uses logic to first render "active" experiments and "completed"
experiments. A "completed" experiment is one in which we've declared a
winner.
Closesforem/forem#17816
[1]:https://github.com/ankane/field_test/blob/master/app/views/field_test/experiments/index.html.erb
With comments, publishing articles, and reactions we had symmetry on two
goals:
- Create a _subject_ (e.g. Comment, Published Article, Reaction)
- Create four _subjects_ within a week.
For page views we only had "Create four _subjects_ within a week." This
PR addes the "Create a _subject_".
To do this required adjusting some tests as they were too specific in
nature (looking at an expected count).
In addition, in consultation with Jennie, I've updated the experiment
order to better reflect some hierarchical importance.
**Rollback considerations:**
This also includes a feature flag that we can explicitly disable if we
overload the application with workers handling reaction goals. Using
`FeatureFlag.accessible?(:field_test_event_single_create_pageview)`
returns `true` unless we explicitly disable this flag.
Related to forem/forem#17673
Related to forem/forem#17669Closesforem/forem#17691
This commit adds three new feed experiment goals:
* user publishes four posts within a week
* user reacts with a "heart", "unicorn", or "reading list" to an article
* user reacts with a "heart", "unicorn", or "reading list" to an article
four times in a week
In addition it adds it to the existing experiment. Adding it to the
existing experiment is acceptable because:
1. We can ignore the results
2. The experiments are structured such that these new goals could be met
with prior data.
3. Both variants are playing by the same rules, so the results relative
to each other are valid.
**There is a nuanced assumption in how we handle reactions:**
This implementation does not count by unique article reactions. However,
most folks will ❤️ , 🦄 , and 🔖 in one swoop; thus
those 3 reactions are all grouped into happening on one day.
Further, it's a reflection of the person taking an action after reading
a post, not how much action on that particular post.
**Rollback considerations:**
This also includes a feature flag that we can explicitly disable if we
overload the application with workers handling reaction goals. Using
`FeatureFlag.accessible?(:field_test_event_for_reactions)` returns
`true` unless we explicitly disable this flag.
Closesforem/forem#17669
* Adding new relevancy lever for privileged reactions
This PR includes four changes:
1. Renaming the variant to remove Jennie's name
2. Starting a new experiment
3. Adding a new relevancy lever
4. Adding a new variant that uses the relevancy lever
1 and 2 is somewhat straight-forward.
For 3, we're looking at having a total of 5 "slots" for privileged user
reactions. Hence the four boundary values.
For 4, we're configuring the granular levers with the following:
- `[-∞..-5)` should have a 0.05
- `[-5..0)` should have a 0.5
- `[0..5)` should have a 0.9
- `[5..10)` should have a 0.98
- `[10..∞)` should have a 1.0
The 20220509-variant.json is a copy of 20220422-variant.json but
replaces the `privileged_user_reaction` lever with the
`privileged_user_reaction_granular` lever.
Below is the diff to highlight the similarities and differences between
the two variants.
```shell
❯ diff config/feed-variants/20220509-variant.json config/feed-variants/20220422-variant.json
90c90
< "privileged_user_reaction_granular": {
---
> "privileged_user_reaction": {
92,96c92,93
< [-2, 0.05],
< [-1, 0.5],
< [0, 0.9],
< [1, 0.98],
< [2, 1]
---
> [-1, 0.2],
> [1, 1]
98,102c95,97
< "fallback": 0.9,
< "very_negative_reaction_threshold": -5,
< "negative_reaction_threshold": 0,
< "positive_reaction_threshold": 5,
< "very_positive_reaction_threshold": 10
---
> "fallback": 0.95,
> "negative_reaction_threshold": -10,
> "positive_reaction_threshold": 10
```
Closesforem/forem#17584
* Update app/models/articles/feeds.rb
Co-authored-by: Josh Puetz <josh@dev.to>
* Adjusting testing logic to reflect latest experiment
* Adjusting negative threshold
Co-authored-by: Josh Puetz <josh@dev.to>
* Adding a field test option for post publishing goals
We have page view and comment view "goals" but have not looked at
publication "goals". We're using these "goals" to help us refine the
feed towards sustainable engagement and community health.
The conjecture on publication goals is that the content on the feed
itself can nudge folks in their decision to publish posts.
Closesforem/forem#17585
* Fixing broken test
Prior to this commit, the SQL fragments included variables that were set
configured at a global level.
With this commit, we're now saying that each lever "knows" what variable
it needs; and providing the means at lever declaration time to "say"
what those variable names are. (e.g. `Articles::Feeds::LEVER_CATALOG`).
Then as part of the variant configuration (in the
`./config/feed-variants/*.json` files) we now include the expected value
of those parameters; which by convention (and coercion) are integers.
This relates to forem/forem#17584 because we want to move from a
privileged user reaction that has two values (`negative` and `positive`)
into four values (`very_negative`, `negative`, `positive`, and
`very_positive`). To do that, we'll create a new lever; but that's for
another pull request.
* feat: update all the paths as a first order of business
* feat: update all the folders to no longer appear under users
* feat: update specs
* feat: update specs
* feat: remove User namespace
* feat: remove route namespace
* feat: path change
* feat: move the menu items to the sidebar
* feat: add a badge
* feat: remove tabs partial
* xs font
* feat: update describe blocks
* feat: update cypress tests
* chore: oops member_manager instead of member_management
* feat: c-indicator
* feat: update the tests
* feat: update the gdpr test
* feat: add the invitations
* chore: align
* feat: update the GDPR constant
* feat: rename to GDPR actions
* feat: add member manager route
* Moving Experiments into AbExperiment namespace
This commit entails two major concepts:
1. Extracting logic out of a worker.
2. Cleaning the conditions in which we convert experiments.
In addition, I revisited some spec names to tidy them up for
clarification and structure.
**Extracting Logic...**
For workers, my preference is that they be a simple router to another
object. A non-worker object, with it's initialize method, can both
better manage instance variables. Further, workers are somewhat "flat"
in hierarchy,
In this case, having the AbExperiment be knowledgeable both in setting
the experiment and handling conversions makes (to me) organizational
sense. For example, the AbExperiment is constructed as a wrapper to the
FieldTest gem. But the RecordFieldTestEventWorker had knowledge of
FieldTest.
With this refactor, it does not have that knowledge. This also provided
an opportunity to replace magic strings with constants.
**Cleaning the conditions...**
This relates directly to and closesforem/forem#17530. I was looking at
the experiments after 2 days, and realized that there shouldn't be
results for
`user_views_pages_on_at_least_nine_different_days_within_two_weeks`;
after all the experiment started 2 days ago, how can we have results
that look at 2 weeks.
By adding the "max" function calls, we ensure a clear boundary of
"before the experiment began" versus "while the experiment is running".
Closesforem/forem#17530
* Update spec/models/ab_experiment_spec.rb
Co-authored-by: Mac Siri <krairit.siri@gmail.com>
* Apply suggestions from code review
Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>
Co-authored-by: Mac Siri <krairit.siri@gmail.com>
Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>
* feat: update all the paths as a first order of business
* feat: update all the folders to no longer appear under users
* feat: update specs
* feat: update specs
* feat: remove User namespace
* feat: remove route namespace
* feat: path change
* feat: update url
* remove name from invite user flow
* remove name from invitation instructions
* update invitations spec
* update specs and invitation actions overflow menu name
* allow users to set name when accepting invite
* First greedy algorithm approach to awarding community wellness badge
* Update app/services/badges/award_community_wellness.rb
I'm sorry I ignored you rubocop. It's still on draft for a reason 😅
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* Update app/services/badges/award_community_wellness.rb
Okay, whatever you say rubocop. This line will go away before merging anyways
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* Use app/queries/comments/community_wellness_query.rb to replace greedy algorithm
* Query indenting + rename positive_reactions to negative_reactions in EXCEPT
* First step towards query spec
* Fix spec and make it more robust
* Slight cleanup/tweaks
* Badge reward & message
* copy edits + spec improvements + 3 days ago min for query
* Cron update to run daily + small tweak for FeatureFlag use
* inline comment tweaks
* Slight reorder of guard checks in service (FeatureFlag related)
* PR review feedback
* Fix specs from static method refactor
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* Remove sustaining member newsletter, and its settings
This removes the concept of sustaining memberships from the system,
and logic related to or dependent on it.
This does not remove the monthly_dues column from the users table (todo).
* Remove unused newsletter setting
Since nothing accesses the mailchimp_sustaining_members_id setting,
it's safe to remove.
* Use destroy rather than delete to ensure settings cache is cleared
We have a callback in Settings::Base to clear the cache after commit,
I assume it's useful to trigger that. This requires destroy, not
delete, to be called.
When we merge this, Forem admins will see the "Admin > Content Manager >
Spaces" section in their Admin area. This will then allow them to
toggle on and off spaces.
Closesforem/forem#17161
* Rename unsubscribe to remove_from_mailchimp_newsletters
* Permanently delete from mailchimp instead of unsubscribing
* Rename method and remove comment
* Don't use i18n because of error message
We don't want to use i18n here because e.title comes from the error response. If a Forem
was using a different locale and Mailchimp for newsletters, they would still receive the
error response in English (because of Mailchimp).
* Refactor and rename a bit
* Rename methods in tests accordingly
* Remove now moot test
* Rename method in other places it was previously called
* Remove unused translation, see 62fee247ee2565e4d3d14919a8636a76ce5ecc9f
* Fix typo and report error if not 404
