* 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>
* 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
* 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>
* 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
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
* Normalize article titles
* Change method name to match what it does
* Expand on allowed emoji ranges
This required a few changes to be in compliance with Rubocop because
that regex is a beast.
In looking at forem/forem#16787, it felt like we would benefit from a
similar approach as we adopted for nullifying blank strings.
This refactor helps pave the way for a "normalize_text_for" method (or
some such thing; naming things is hard).
This change also involved introducing a `describe` block into the
associated spec.
* feat: remove the default email and cobine the periodic digest and the contact email under the Email section
* refactor: rename the email_link to contact link and use the contact_email as a default and fallback to the ForemInstance.email
* chore: alignment
* feat: use the contact_email helper
* feat: move the contact_email to the ForemInstance model
* feat: use ForemInstance.contact_email instead of the application helper method
* removed the application Helper
* feat: set the dafault on the contact_email
* fix: cypress tests
* Update app/lib/constants/settings/general.rb
Co-authored-by: Michael Kohl <me@citizen428.net>
Co-authored-by: Michael Kohl <me@citizen428.net>
* 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>
In my quest to find `where(attribute: value)` in controllers and even
services, I stumbled upon this pattern.
As of this commit (and prior) an Article's associated user is it's author.
* test: whitespace unicode characters cannot be used as titles or tags
* feat: add localized error message
* refactor: use localized error message
* fix: whitespace unicode characters cannot be used as titles or tags
* chore: fix locale after merge
* refactor: fix indentation
* Fix spelling of prohibited in method names
Additionally, rubocop removed a redundant user_id validation since
Article belongs to user.
* Fix spelling
Missed one method call on the same line (fixed the first of two
instances needing changes)
* add failing test cases
The reorganization to remove let was due to a limit on nesting rspec
contexts (it inside context inside describe inside describe when
trying to use different titles in let blocks in contexts)
The first test was clarified (the "U+202D" string that looks like the
code for a unicode point is valid, but the character \u202d is
expected to be invalid
The remaining two tests are based on the feedback I'd given in the PR,
failing because we don't assert title is present after removing
unicode whitespace, and we don't actually set the title (gsub is
non-destructive, returning a value).
* Set title to sanitized title
contains_prohibitied_unicode_characters? was using == (which is not a
good match for strings to regexes), use =~ instead
Change invalid example characters in titles from \u202d (bidi override) to
\u200a (hair space)
Assert that the empty title is blank and can't be blank (even though
it contains no prohibitd characters after replacement)
* change the definition of prohibited characters
From the description, it looks like "unicode space characters" was the
desired rejection set. It was unclear what the existing regex was
matching on (I couldn't get the expected examples to match correctly).
Given the intent, I select "unicode space property, except the ascii
space character", which may _also_ be incorrect but passed the tests.
* Remove now-invalid expectation for presence of user id
Since this was redundant (belongs_to user), we no longer will validate
presence of user id, and we should not test for it.
* Only reject bidirectional text controls
The original issue pointed to the BIDI controls as problematic.
While they called out other "whitespace" characters, as we accept a
wider range of input languages, the need to handle non-printing
punctuation (for example a group separating space for some asian
languages).
It's possible a wider list of characters should be added - if that's
the case I suggest this regex and the sanitization be moved to a
standalone class, and each of the recommendations in
https://www.w3.org/TR/unicode-xml/#Charlist be checked for
applicability.
* Check for blank title after sanitizing disallowed characters
Since validate_title modifies the title, and doesn't set error
messages on the model itself, check for non-empty title _after_
potentially removing any disallowed characters.
* Remove invalid characters before validation
Remove the validate_title and contains_prohibited_characters methods,
and center all logic on the remove prohibitied unicode characters
method.
Remove unneeded and unused arguments (input string is always title,
replacement is always removal/empty string).
* handle case when validating null title
If the title's nil, we don't want to call match? since it will fail.
Title will sometimes be nil when validating.
Co-authored-by: Ben Halpern <bendhalpern@gmail.com>
Co-authored-by: Dan Uber <dan@forem.com>
* Refactoring to leverage an Article scope
I've been looking at the queries that involve filtering articles based
on scores. There's several places that seem to be close to consistent,
but have some nuanced difference.
This refactor consolidates one of those "almost the same" cases.
* Bump for travis
* Refactoring to use StringAttributeCleaner
In #15281, we introduced the StringAttributeCleaner. Let's use it!
* Adding StringAttributeCleaner for User properties
* Parameterizing callback for attribute cleaning
* Reading and applying the example for attribute cleaner
* Refactoring Spam Handler
There's considerable repeated logic between checking spam for an article
and spam for a comment and user.
This attempts to send things through channels that are similar and close
in organization.
* Fixing broken spec
* Fixing spec around recent user
* Update app/models/reaction.rb
Co-authored-by: Michael Kohl <citizen428@forem.com>
* Update app/models/reaction.rb
Co-authored-by: Michael Kohl <citizen428@forem.com>
* Consolidating new user query logic
Prior to this commit there were two separate queries around new user
logic. With this commit, we're changing the logic to repurpose a site
wide setting.
* Generalizing a previously specific message
* Fixing method name
As part of a recommended refactor, I extracted a method, then renamed
it. I failed to account for that renaming.
This commit fixes that.
Co-authored-by: Michael Kohl <citizen428@forem.com>
* Favoring dependent: :delete_all over :destroy
The `dependent: :destroy` callback is slower than `dependent: :delete`
and `dependent: :delete_all`. We need only favor the `dependent:
:destroy` when there's callbacks that happen.
In the case of :destroy, ActiveRecord instantiates each object and then
runs destroy. Whereas in the case of :delete, ActiveRecord issues a SQL
command to delete the related files.
It is often "safer" to use :destroy, as it guarantees that you'll
instantiate the record and run it's callbacks. But sometimes you have
to go with the speed of SQL.
This relates to #14140. Note there is still more to consider, but given
that we're looking at moving from :destroy on all of an article's page
views to :delete, we might buy enough time in the callback.
* Removing redundancy of article destroy
Prior to this commit, `before_destroy_actions` called the `bust_cache`
method which in turn called `touch_actor_latest_article_updated_at` but
`bust_cache` did not pass the destroying parameter. Then
`before_destroy_actions` immediately called
`touch_actor_latest_article_updated_at` with `destroying: true`.
With this change, we remove one of those duplicate calls.
* Fixing specs regarding relationships
* Fixing specs regarding relationships
* Refactoring and extending article spam behavior
Prior to this refactor, we were checking an Article's title for
spaminess. This change now checks the Article's title and
body_markdown.
In addition, the encapsulates the regular expression implementations in
favor of asking the Settings::RateLimit class to determine if the passed
in text is "spammy".
Furthermore, instead of having lots of inline logic within the article,
this refactor introduces a `Spam::ArticleHandler` class. This helps
tidy up the already busy Article specs by delegating the business logic
of Spam handling to it's own class.
There are further refactors for Comments and Users that would follow
this pattern, but this change is more important to get out the door.
Closes#15396
* Adding comments to clarify behavior of spam handling
* Adding missing expectation to spec
This commit removes a handful of magic numbers and instead relies on a
constant.
This has a small impact in that the Basic feed will now return 50
articles instead of 25. However, normalizing the feed pagination window
size helps reduce some oddities in reporting.
In addition, this might be something we consider giving administrators
the ability to set (with default options, because we shouldn't allow
page sizes of 10_000 as that's a massive memory hog).
Related to #14709
* Caching sum privileged reaction scores
This relates to work on improving the feed. Namely that in the current
proposal in PR #15240 I'm not accounting for how privileged users have
given feedback on an article.
With this change, I will now have a cached value on the articles table
that can be a proxy for how the privileged users have reacted.
In addition there's a small refactor that moves a constant to the
correct scoping object.
Dependent on #15283.
* Fixing method name to `exists?`
Prior to this commit, I was using `exist?` which doesn't work for
ActiveRecord::Relation objects.
* 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>
* Separate followed and "anti-followed" tags
* Minor code change
* Indicate "anti-followed" tags in sidebar
* Fix UpdatePointsWorker spec
* Cries in Arel
* Use antifollow scope more selectively
* Add spec for not_cached_tagged_with_any scope
* Add explanatory comment
* Remove unused route
* Update comment
* Remove unrelated change, figure out why > 1 later
* Need more ☕
* Avoid false positives in spec
* Change handling of ingored tags in sidebar
* Add explanatory comment
* Move logic to determine noindex from view to model
This brings attention to the code and allows for faster unit tests.
I expect we'll be modifying this soon to deemphasize the code tag as a
marker of quality.
* Replace "5" constant with home feed minimum score
* Remove check for "code" tags in low quality article check
This made sense for DEV and other devrel communities, but is no longer
necessary for Forems.
* Add UserExperience setting for minimum index score
- add setting, I used `index_minimum_score` to match the
`home_feed_minimum_score`, `minimum_index_score` feels natural but
breaks the pattern.
- use this to determine whether to include article in the sitemap
- use this to determine whether to add noindex meta to article show page
* Remove unneeded tests
Since there's no longer any check for <code> tags in the body, omit
those tests (which passed anyway).
* Add new setting to admin page
* fix: artiles from the same feed can be fetched from multiple accounts
* undo unintentional changes to schema.rb
* modify: article_spec and import_spec tests
* fix: failing test
* modify: import_spec:213 for user.setting
* fix: failing test import_spec:213
* add: unique index on articles canonical_url where published
* revert: schema.rb
* modify: unique url error message to include admin email
* modify: custom error message
* modify: .update to .update! (bang)
* Simplify logic around generated article description
@citizen428 helpfully pointed out ActiveSupport's String#truncate
method, and I had read the processed_description method body
immediately before and knew it could benefit from using truncate.
When the body text is present, if it's long, truncate to 100
characters (with trailing ellipsis as needed), removing newlines.
When the body text is absent, populate with a generic "post by user"
message.
* Change magic description length to match the original behavior
Since we were previously slicing [0..100] we would get 101 characters, and then
appending three dots makes for 104 total.
There is a longish description passed in the articles api request test
that shows "part of the body as a description" that failed to match
with the first pass.
* Create DiscussionLocks
* Fix specs
* Update nullify_blank_notes_and_reason
* Update before_validation call
* Updated DiscussionLockPolicy for clarity
* Move permitted_attributes to a constant
* Update route
* Apply suggestions from code review for frontend
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* Add title tags
* Wrap unlock confirm in main element
* Wrap flash messages up in div
* Actually fix title tags
* Hide comment reply button when discussion is locked
* Add E2E tests
* Try to fix E2E tests
* Cypress...you work locally but not in CI...why!?
* PR feedback
* Update E2E tests
* More E2E updates 😭
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* Add HairTrigger spec
* Add weights to columns tsvectors
* Add correct sorting for search results
* Remove Timecop.travel
* Changed order of columns in trigger based on weights
