* Article query spec for scheduled articles
* Added scheduled article badge on the user dashboard
* Added published_at field to editor options
* Accept and validate published_at from editor
* Refactor published_at validation
* Allow 1-minute difference in published_at
* Notice on an unpublished article page
* Added specs for 'Click to edit' link on scheduled article preview page
* ContextNotification model
* Articles::Publish worker
* Added specs for articles publish worker
* Schedule publish articles worker
* Added tests to check for scheduled posts in feeds
* Don't allow managing scheduled articles
* Don't send notifications for scheduled articles
* Set published_at in Articles::Updater when publishing
* Published_at value in post options
* Pass timezone and set published_at accordingly
* Limit setting published_at to the future
* Readonly published_at for articles that were already published
* Chagning published_at format in editor v1 (start)
* Changed published_at format in frontmatter, specs
* Added specs for updating published_at from frontmatter
* Fixed accepting past published_at for articles published_from_feed
* Enabled published_at validation: don't allow updating published_at for already published articles
* Validate published_at on create
* Added a spec for updating published_at for exported articles
* Fixed specs related to creating articles with past published_at
* Fixed specs related to past published_at for articles
* Added a hack so that admins would be able to update published_at
* Switch button text schedule/publish when changin publishedAt
* Fixed saving published_at with timezone
* Added a feature flag for scheduling articles
* Default text in markdown editor depends on feature flag
* Enable article editor cache again
* Fixed the default value in the markdown editor
* Fix sitemaps spec
* Removed tooltip
* Fixed articles update specs
* Added missing locales
* Fixed article create specs
* Fixed spec
* Removed commented code
* Returned enabling extensions in the schema
* Returned accidentally deleted constraint
* Make articles query spec more stable
Co-authored-by: Jeremy Friesen <jeremy.n.friesen@gmail.com>
* Removed commented code
* Removed unused code
* A clearer policy
Co-authored-by: Jeremy Friesen <jeremy.n.friesen@gmail.com>
* Use StringInquirer for article current state
* Added a note and todo to articles factory past trait
* Remove duplicated PropType
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* Refactor query in the Articles::PublishWorker
Co-authored-by: Mac Siri <krairit.siri@gmail.com>
* Refactor articleForm.jsx
Co-authored-by: Mac Siri <krairit.siri@gmail.com>
* Removed specs that are no longer relevant
* Removed useless onKeyUp on a hidden input
* Refactored articleForm
* Hide scheduling from post options when published_at is readonly
* Run sends notifications worker every 5 minutes instead of every minute
Co-authored-by: Jeremy Friesen <jeremy.n.friesen@gmail.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Mac Siri <krairit.siri@gmail.com>
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
In the general sense we have an existing `order by` that follows the
following format: `order(x) = f(x) * rand() ^ ( 1 / g(x))`
For the present [20220603-variant-b][1], `g(x)` is the article’s score.
And `f(x)` is 1.
This pull request introduces a few named variations for the above
`order(x)` formula.
The three named formats each use a different `g(x)` function; namely
`greatest(0.1, ln(1 + greatest(0, public_reactions_count)))`. The more
public_reactions the article has the closer to zero the resulting
positive number will be. And raising a random number, with range
`(0..1)`, to the inverse of that number will tend to mean that artciles
with many reactions will tend to group towards the higher end of the
random range, whereas articles with few reactions will have a more even
distribution across the range.
Further the `ln` function helps dampen the weight we give to reactions.
The variation within these three order levers is on the `f(x)` portion:
1. Using `published_at`
2. Using `last_commented_at`
3. Using a mix of `published_at` and `last_commented_at`
In all cases, those dates are converted to an integer. A date 2 weeks
ago will result in a number less than a date 1 week ago which will be
less than a today.
The idea for the mix of `published_at` and `last_commented_at` is to see
a mix of things where some “older” articles (from the underlying result
set) will get a chance to show up earlier in the feed.
My rational for this order structure is because the `RANDOM() ^ (1 /
score)` is showing possible success in a current experiment. And in
past trials long past, it rose to the top as a likely and viable
contender.
These new `order_by` lever patterns introduce three things:
1. A variable based on a date in time
2. A dampening of what can be a large number; (we have scores in the
publication range that are between -200 and 750 so I think it is
important to apply a dampening. For example `(0.1 ^ (1/100)) =
0.977` and `(0.01 ^ (1/100)) = 0.955`; in other words these large
scores aggressively pushed their corresponding posts to the top of
the relevance; even as we weren't explicitly querying for those
scores.
3. With the score being quasi-magical, it doesn’t make sense to sort on
that value but to instead sort on a property that represents what
we’re setting as our experiment goals (e.g. publishing something or
commenting on something and/or reacting to something)
Note: it would be feasible to extract these into a singular order lever
that we would configure. But that's a future concern.
Also note, none of these are yet part of any experiment, but are instead
being "queued up" to add to our available corpus.
Closesforem/forem#17834
Related to:
- forem/forem#17827
- forem/forem#17826
- forem/forem#17833
- forem/forem#16128 :: for past order queries to seek as inspiration
[1]:https://github.com/forem/forem/blob/main/config/feed-variants/20220603-variant-b.json
The commit includes three changes that work to allow us to "seed" the
randomization. When using the same seed, and randomizer should/must
return the same sequence of numbers.
First, I added the "seed" parameter to the variant query. If you want
the same sequence of random numbers from query to query, pass the same
seed. Otherwise, we'll use a Ruby generated random seed.
Second, I added a virtual column to the virtual `article_relevancies`
table. If you provide a seed, and call the query twice, the first row
in each of `article_relevancies` will have the same `randomized_value`,
likewise the second row, etc.
Third, I amended the existing `order_by_lever` that had a `RANDOM()`
postgresql function call. I replaced that with the `randomized_value`
which is computed based on the provided seed.
Fundamentally the idea is to allow for randomness but introduce possible
repeatability; a hard thing considering that some of the inner selection
criteria is not fixed (e.g. number of reactions can change from moment
to moment).
I wrote a [complimentary blog post][1] to further explain what's
happening.
Related to forem/forem#17826
[1]:https://takeonrules.com/2022/06/03/adding-reproducible-randomization-to-sql-queries/
* Adding more context to `/admin/abtests`
Prior to this commit, the `/admin/abtests` was fully rendered by the
field_test gem (see
https://github.com/ankane/field_test/blob/master/app/views/field_test/experiments/index.html.erb
and
https://github.com/ankane/field_test/blob/master/app/views/field_test/experiments/show.html.erb).
With this commit, we're pre-pending our own
partial (e.g. `app/views/field_test/experiments/_experiments.html.erb`)
in the view paths; which means instead of rendering
https://github.com/ankane/field_test/blob/master/app/views/field_test/experiments/_experiments.html.erb
we render our newly created
`app/views/field_test/experiments/_experiments.html.erb`)
Why the ugly antics in the view? As I'm not fully certain if this will
"meet" the full needs of those monitoring the experiments. I'm also
constructing this to most closely match the spreadsheet that has been
assembled for tracking this information.
Why introduce another caching layer for the variant assembly? The
original caching
layer (e.g. `Articles::Feeds::VariantAssembler.pre_assembled_variants`)
is in place to provide the quickest access to the assembled variants.
But due to the nature of the implemented view, we are needing a
programmatic representation of all variants. And the added Rails cache
is there to minimize disk reads.
Closesforem/forem#17820
* Update app/models/articles/feeds/variant_assembler.rb
Co-authored-by: Ridhwana <Ridhwana.Khan16@gmail.com>
* Update app/models/articles/feeds/variant_assembler.rb
Co-authored-by: Ridhwana <Ridhwana.Khan16@gmail.com>
Co-authored-by: Ridhwana <Ridhwana.Khan16@gmail.com>
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
* added ability to assign moderator role via admin/super admin
* create Mod User in seeds
* add Cypress specs
* fix failing Cypress tests
* fix failing specs by moving test Mod user to the end
* nudge Travis
* nudge Travis
Co-authored-by: Arit Amana <msarit@gmail.com>
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
The [`ArticlePolicy#moderate?`][1] method answers the questions around article
moderation.
At present this change does not introduce any significant changes (aside
from the JS logic loop). The next "step" is to adjust the
`ArticlePolicy#moderate?` method to test if the user has a moderator
role.
Further, we'd need to see what is available on the action panel.
Related to forem/forem#17606
[1]:5ac3216a5a/app/policies/article_policy.rb (L160-L174)
There are 2 things going on in this pull request:
1. Reducing duplication of knowledge, by favoring constants found in
Reaction.
2. Creating and enforcing the policies regarding the different
categories of reactions.
I chose to conflate these two as I was working on developing the
ReactionPolicy's finder method.
Closesforem/forem-internal-eng#454
Related to forem/forem#17628
* 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 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
You had a good ride, but your successor, the
`Articles::Feeds::VariantQuery`, is doing well in production. And has
been for quite awhile.
You helped us get to a better spot in regards to the feed algorithm, but
your time has come to rest as a memory in our git history.
* 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
* Update sort to have nulls at last
* Updating Article.sorting by published to only include published
Co-authored-by: Jeremy Friesen <jeremy.n.friesen@gmail.com>
* Allowing VariantQuery for Feed Generation
Apologies for the breadth of this pull request, I had considered many
small commits, but felt that would've been more effort for the value
provided.
This commit includes the following:
- Documentation updates to the feed variant (though not the final pass)
- Renaming and adding RelevancyLevers that help differentiate
- Adding RelevancyLever#range to provide documentation
- Reducing redundent controller logic by making a
`Articles::Feeds.feed_for` method.
- Adding some configuration validation for RelevancyLevers
- Adding constants for better clarification
- Testing unhappy paths for feed configuration
- Adjusting the module namespace of some objects
- Exposing top-level configurations for variants (along with their
defaults)
- Creating the VariantyQuery that at present inherits from the
`Articles::Feeds::WeightedQueryStrategy`
As implemented, we can deploy this code to production without using the
new VariantQuery. Once we toggle on the
`:feed_uses_variant_query_feature` FeatureFlag, it will switch to using
the VariantQuery. The VariantQuery's two variants and the
internal configuration of `Articles::Feeds::WeightedQueryStrategy`
produce the same query.
The goal of this factor is to allow for a quick on and off toggle of the
feed query; to ensure that what we introduce remains performant.
- Closesforem/forem#17272
- Closesforem/forem#17276
- Closesforem/forem#17216
In addition, I will be recording a code-walkthrough and linking that
recording to the pull request.
* Apply suggestions from code review
Co-authored-by: Mac Siri <krairit.siri@gmail.com>
Co-authored-by: Mac Siri <krairit.siri@gmail.com>
We avoid busting a user cache for 15 minutes but the tag cache is 5
hours. So we want to "nudge things along".
What this is trying to solve is the server side rendering of whether we
hide or show a button.
Related to forem/forem#17324
Related to forem/forem#17119
**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
* 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
* Conditionally disabling Listings
Prior to this commit, we were assuming everyone still had the Listing's
feature turned on.
With this commit, we move from the "implicitly on" nature of
`FeatureFlag.accessible?` to the "explicitly off" nature of
`FeatureFlag.enabled?`. The key differentiator is that we are not
"pushing out a feature flag setting to other forems." Listings now
become an "opt-in".
This requires a few things:
1) Ensure that DEV.to enables the `:listing_feature` before we deploy
this branch.
2) Communicate this change to other Forem adopters and provide a
timeline for when we'll deploy this change.
3) Documentation. Given that we have this feature, but it will be
disabled by default, we want to look into communicating what it is
and how to enable it.
Further, I chose to do the least effort change on our specs. Namely to
treat the specs as though someone fully enabled the listing feature.
This pull request is a "competetor" to #16423.
Closesforem/rfcs#291
Relates to #16420 and #16461.
* Favoring only setting feature for specific tests.
* Adjusting cypress to avoid listing check
* Removing redundant test based on feedback
* Fixing feature spec
* Fixing cypress test
* Encapsulating the "why we hide/show" something
Prior to this commit, we were relying on our views/templates to
understand why it might want to append the "hidden" dom class. That is
not ideal, because it required repeating knowledge about the conditions
of the policy outside of the policy's domain.
With this commit, we're removing a duplication of knowledge; or more
appropriately asking the steward responsible for the knowledge to
produce the answer.
* Refining the note to not describe a course of action
* Adds data to the member column in Member Index View
* Adds roles, statuses, and latest activity to Member Index views
* Adds the list of orgs a user belongs to in Member Index view
* Updates the image URL to link to each users member detail view
* Adds comment_suspended to user current_status and makes roles prettier
* Fixes bullet warning and encapsulates org logic in method
* Properly displays today and yesterday in words
* Refactors #current_organizations to use ternary
* Updates user.rb Member Index View-related methods
* Adjusts the styling of user roles and adds tooltip to roles
* Updates Member Index View to be mobile-friendly
* Updates userIndexView.spec.js
* Updates capitalization within userIndexView.spec.js
* Updates Member Index View per mobile designs and feedback
* Adds member status logic inline with SVGs
* Updates the status column for mobile layout
* Adjust current_role tooltips for Member Index View
* Adds organization name tooltip to member orgas
* Adds organization pics to Member Index View and cleans up helpers
* Addresses PR review feedback (Thanks, Suzanne!)
* Uses CSS in place of SVGs for member statuses
* Adds E2E test for and removes link to org page from Member Index View
* Addresses PR feedback comments by refactoring Member Index View code
* Removes leftover code in user.rb
* Update method that tooltips rely on
* Spells regular correctly :
* Adds a spec that tests Admin::UsersHelper#format_last_activity_timestamp
* Adjusts #cascading_high_level_roles logic and roles
* Adds a spec for #cascading_high_level_roles
* Adds a #last_activity spec to the User model
* Adjusts code a bit per PR feedback. Thanks, Ridhwana! :)
* Updates specs for Member Index View helpers
* Adds Timecop.freeze to help with timestamp failure in user_spec.rb
* WIP - Conditional rendering of "Create Post" link
This PR builds on the conversation from forem/forem#17056 and moves in a
slightly different direction.
Important in all of this is that the ability to create a post is
enforced on the server. If the "Create Post" button were to be visible
but the user couldn't create a post when they clicked the button, they
would get an authorization error (or some such response).
This PR posits a different and perhaps competing approach to
forem/forem#16606. This PR provides a general approach in which we add
class attributes in our HTML erb files.
Note: I have not included Cypress tests as I don't want to yet commit
that time. I'm also wondering if this is the "right" thing to do. I
definitely think we want to add some JS tests. But could we do JS and
unit tests? (How do we reach consensus regarding our test approach?)
Again, thank you for the conversation and expect an even more "complete"
PR after we have our discussion.
* Extracting AsyncInfo model to ease testing
* Renaming forbidden to visible
* Adding cypress test to assert no 'Create Post'
These are always treacherous. What happens when we rename the button?
The test will continue to work.
* Apply suggestions from code review
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* Switching to pack file for policy
Follows on [Suzanne's comment](https://github.com/forem/forem/pull/17076#issuecomment-1088567286)
* Update app/javascript/packs/applyApplicationPolicyToggles.js
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* transliterate when generating attribute names to avoid emptiness
If you enter a non-ascii (non `\w` matching) string as the profile
field label, the attribute name is the empty string.
This causes problems outlined in #16391
To avoid losing user provided data, transliterate (Sterile is the same
tool we're using for Article#title_to_slug) before matching against
the word regex.
If we persist an empty string, or persist a non-`\w` name, the
coordinating regex in Profile::ATTRIBUTE_NAME_REGEX will leave a nil
match and raise NoMethodError (the method missing is for
match[:attribute_name] when match was nil, not Profile - that's the
next commit.
* Guard against nil matches
If an attribute name doesn't match the regex, the match is nil, and
trying to access (nil)[:attribute_name] raises a NoMethodError.
If there was no match, assume profile does not respond to the
selector, and don't handle it in method missing.
* Ensure generated attribute name is valid before saving
This raises a validation error if the generated name (from the label)
would be empty.
It's not an optimal error message (since the user can't see the
internal attribute name) but it prevents persisting broken/empty data
* actually raise error when validating
validate/valid? only return true or false (and set errors on the
object). In order to reject the creation, we need to raise a
validation error, not only call validate. I think this is because the
execution of before_create hooks happens after validation (which is
why the validation was only checked on update, not create).
* Generate an attribute name completely independent of the field label
This prevents mistakenly labeling a field "Class" or "Association" or
one of the other hundred public methods an AR model like Profile
exposes. Since attribute_name will be passed to `profile.public_send`
we really shouldn't build selectors from user supplied inputs.
* Use the admin supplied label in the sidebar
The profile decorator is used in the Users#show page to populate the
sidebar fields. Don't use the attribute name (which we mangled during
creation, and now generate randomly) as the label, use the label.
* Make the label lookup null safe, and filter attributes more
* Update data update script to not expect predictable labels
This is low impact since it ran in july, but we no longer know what
attribute name a label will create.
* Fix moderator spec
"Test Field" label no longer predictably generates :test_field as an
attribute name. Ask the field what it's name is before asking profile
about it.
* Fix profile preview card request spec
Remove the assumption that profile responds to a method name based on
the label for Work and Education fields.
* Update old DUS and its test
This can probably be archived
* Update profile spec to use fields generated attribute names
We used to "know" how attributes were generated from labels. Now we don't.
* update e2e seeds for profile field change
* Don't expect attribute name to be based on the label
* expect created profile fields respond to their attribute name
* Update system test
The label, not the attribute name, is shown on the profile form (the
field has an id related to the attribute name, but the view shows the
mutable/human-readable label).
* Keep the field title lowercase when sending the json preview card
The userMetadata component expects "work" and "education" to be
attributes of the metadata, but the ui_attributes_for() method was
titlizing these (for display).
Ideally we wouldn't have "special purposed" these two field names, but
they're there.
* update profile field by attribute name, not based on label
* Update profile field removal assumptions
We don't know what the method selectors will be, we have to ask.
* remove old test
* Update translations for Education and Work
Since the ui_attributes_for(area:) now gives the label, not the
attribute, we need to match the label of the profile field.
Note to self: this exposes an issue in localizing the custom profile
fields (probably a bigger problem for large, international communities
like DEV than some others, but trying to match static translation
files against user-modifiable database records seems like a problem
we'll see again).
* Empty commit to retrigger buildkite
* Remove profile field migration update scripts
Cloned the specs from the other "remove unused scripts" script.
* Remove unused scripts
The data update script removes the entry from the table (recording
that these have run) - we also want to remove the files (preventing
them from running again).
* remove unneeded spec for removed file
* when translation for header area field not found, use the title
Only Work and Education already have keys in the yml translation file,
and there's not a great (or easy?) way to make multiple translations
on these fields right now.
Since an admin can create a new field, and assign it to the header
area, we can't assume the code has a configured translation key for
this field.
Fallback to the title (we do this in another context already) if
there's no translation.
* PR feedback: Avoid n+1 query for labels
the original implementation of "label_for_attribute" had an n+1 query
looping over each matched key.
Follow suggested improvement and pull labels and attributes at once
from the db and modify the returned hash.
* Downcase title before looking for translation key
This avoids putting "odd" capitalized keys into the yml translation
file
Revert addition of "Work" and "Education" to the users files.
* use a let binding for duplicated test data
* Update app/models/profile_field.rb
prefer SecureRandom.hex for a dashless uuid (instead of removing the dashes).
Co-authored-by: Jamie Gaskins <jgaskins@hey.com>
Co-authored-by: Jamie Gaskins <jgaskins@hey.com>
Prior to this commit, when we change attributes for a user we might
resave each article. The purpose of the resave is to update some of the
cached attributes of an article.
This removes some redundant code. The following removed code has
an `article.save` call. Which, the callbacks in the article (see below)
already clear the cache.
```ruby
def resave_articles
articles.find_each do |article|
if article.path
cache_bust = EdgeCache::Bust.new
cache_bust.call(article.path)
cache_bust.call("#{article.path}?i=i")
end
article.save
end
end
```
8e6981aac5/app/models/article.rb (L164)8e6981aac5/app/models/article.rb (L881-L888)
This was discovered in triaging forem/forem#17041
There are three major things occurring in this pull request:
1. Renaming `Article#update_cached_user` to `Article#set_cached_entities`.
2. Reducing an organization's direct knowledge of which of the org's
attributes an article caches.
3. Removing duplicate calls to update the article associated with the
organization.
For renaming to `Article#set_cached_entities`, the prior method implied
we were updating the persistence layer. However, we were not making any
save nor update calls. This rename should clarify intention.
For reducing knowledge, the comments for
`Article::ATTRIBUTES_CACHED_FOR_RELATED_ENTITY` should explain the details.
And last, removing the duplicate calls; we had three methods that were
attempting to build and update the `Article#cached_organization`'s
value.
Closesforem/forem#17041
