* add default placeholder, remove focus on first load
* fix some bugs re autofocus and mouse click to select
* allow custom selected styles to be passed in
* operate on objects with name property rather than plain strings
* WIP main functionality in place
* set default selections, allow a max to be placed on selections
* switch help context
* bug fixes to edit mode, static suggestions
* make sure suggestion resumes when edit begins
* cleanup and docs
* update existing form test
* add component tests
* add more component test cases
* refactor max selections flow, ensure default tag data only loads once
* stop removing combobox properties now the input stays visible
* add max selections test
* refactor
* make sure input refocus happens after blur event
* update cypress tests
* some small renames and doc changes
* only fetch exact matches from added tags
* fix test, update dark theme background
* set a max height on the popover, and ensure options can be scrolled into view
* woops - max height
* Update app/javascript/article-form/components/TagsField.jsx
Co-authored-by: Ridhwana <Ridhwana.Khan16@gmail.com>
* refactors
* stop dropdown from flickering
* use ButtonNew
* remove redundant variant
* nudge PR checks
Co-authored-by: Ridhwana <Ridhwana.Khan16@gmail.com>
The fix might be a bit of a work-around for a more general approach, but
we're fighting against the magic of the points vs. explicit_points
tension as defined in the [Follows::UpdatePointsWorker][1].
That is to say, it's possible (and happens) where I have assigned an
`explicit_points` of 1 for a tag, but the calculated `points` are less
than 1. The calculated `points` are what we send forward in the
[AsyncInfoController][1] (by way of the
[UserDecorator#cached_followed_tags][3] method).
In DEV, I ran the following queries:
**User assigned points `>= 1` but calculated `< 1`**
985,352 records
```sql
SELECT count(id)
FROM follows
WHERE followable_type = 'ActsAsTaggableOn::Tag'
AND points < 1
AND explicit_points >= 1;
```
**User assigned points `>= 1` but calculated `>= 1`**
5,366,478 records
```sql
SELECT count(id)
FROM follows
WHERE followable_type = 'ActsAsTaggableOn::Tag'
AND points >= 1
AND explicit_points >= 1;
```
**User assigned points `>= 1` but calculated `>= 0`**
0 records.
```sql
SELECT count(id)
FROM follows
WHERE followable_type = 'ActsAsTaggableOn::Tag'
AND points <= 0
AND explicit_points >= 1;
```
**User assigned points `<= 0` but calculated `> 0`**
1,146 records
```sql
SELECT count(id)
FROM follows
WHERE followable_type = 'ActsAsTaggableOn::Tag'
AND points > 0
AND explicit_points <= 0;
```
Synthesizing that, almost 1 million tag follows of the total 6.3 million
are not showing up in the user's left navigation. And 5.3 million are
roughly correct.
In adjusting the logic, we're now going to have more tags showing up in
the left. And just over 1,000 might now show up that would be unexpected.
Closes#14937.
[1]:c63e0b629e/app/workers/follows/update_points_worker.rb
[2]:c63e0b629e/app/controllers/async_info_controller.rb (L40-L65)
[3]:c63e0b629e/app/decorators/user_decorator.rb (L23-L34)
* Let override of ToS and CoC show in onboarding
Prior to this commit any changes to the terms or code of conduct were
not reflected in the onboarding links.
With this commit, I'm leveraging a newly created method that first
checks if there's a page for the given slug. If there is, use that
page's copy.
To test:
- Overwrite default /terms page on a Forem
- Navigate to /onboarding or sign up as a new user
- Click on Terms of Use link text
- Review Terms of Use
Closes#15296
* Adding spec around not passing a block
* Removes the Getting Started section and related code from the config
* Removes the admin_manages_configuration_spec.rb
* Removes "Required" tags from Config
* Reverts removal of activateMissingKeysModal
* Removes mandatory.rb
* Remove special handling of "connect" feedback by name
The special casing was related to "connect" feedback having both a
reporter and an offender. Check for offender instead.
Additionally, there was special casing in the controller to rate-limit
connect feedback separately from other channels. Since connect doesn't
exist, we should not need this.
There's a small bit of functionality (when I post to feedback_messages, the
number of feedback messages increases) that was removed from the test
case, we can add that back (and "connect" type, and
offender_id attributes) since it looks like it might have been a
useful assertion.
* Add back feedback message controller creates feedback message case
This was removed in the last commit because it was in a "connect" chat
channel context, but the basic "should persist a record" test was
otherwise valid. Submit an abuse-report rather than a connect message
report.
* typo
feeedback, woops.
* Select max from a subquery of two things
max(int, int) not a function, says postgres, once it stopped yelling
about parenthesis balancing.
* prefer greatest to select max from subquery
* Ensuring that we don't divide by zero
Adding one to a "always-ish greater than or equal to 0" value. This
should resolve a flakey test and a seemingly erratic production error.
Resolves https://app.honeybadger.io/projects/66984/faults/83596547
* Favoring min over adding 1
Due to syncrhonization antics, let's not assume time is consistent
across servers.
Timey Whimey Wibbley Wobbley
* Update app/services/articles/feeds/weighted_query_strategy.rb
Co-authored-by: Daniel Uber <djuber@gmail.com>
Co-authored-by: Daniel Uber <djuber@gmail.com>
* Ensuring we don't track views of author or unpublished
Prior to this commit, I was surprised to learn that we:
1) Tracked an author's view of their article.
2) Tracked views of an unpublished article.
This came up from a Forem creator asking if they could reset the view
counter. Or trigger the reset on publication.
I think a general business logic policy of don't track views for the
author and don't track views for unpublished articles is a reasonable
default.
Were we to pursue the clear views on publication, we'd need to consider
something that went from unpublished -> published -> unpublished ->
published. Without a more explicit state machine, triggering a
busineiss logic behavior seems a bit unexpected.
In other words, I wrote an article. There are 20 views when I realize
that I need to unpublished it. I make the changes in the unpublished
state, and re-publish. I'd assume that those 20 views would still be
"recorded" and counted towards my article's view counts.
* Adjusting condition structure
Prior to this commit, the `if` clause was rather far to the right. This
helps make the if clause more pronounced.
We are loading the badge for each included tag in the
Search::TagSerializer and seeing warnings from bullet
```
Bullet::Notification::UnoptimizedQueryError:
GET /search/tags?name=ta
USE eager loading detected
Tag => [:badge]
Add to your query: .includes([:badge])
Call stack
/home/travis/build/forem/forem/app/serializers/search/tag_serializer.rb:5:in `block in <class:TagSerializer>'
/home/travis/build/forem/forem/app/services/search/tag.rb:11:in `serialize'
/home/travis/build/forem/forem/app/services/search/tag.rb:7:in `search_documents'
/home/travis/build/forem/forem/app/controllers/search_controller.rb:54:in `tags'
/home/travis/build/forem/forem/app/lib/middlewares/set_time_zone.rb:10:in `call'
```
Follow the advice, now when multiple tags are in the result set,
having multiple badges, badges are loaded only once (at query time)
and not one by one (at serialization time).
* Allow for skipping navigation link creation
This commit provides a possible solution for preventing the re-creation
of a navigation link deleted by a Forem creator.
What I need is a discussion around the life-cycle of the application
installation and updates.
In particular, does this provide a robust enough mechanism for resolving
the issue at hand?
_Note: it pains me to ask about a user's role, but this is provided as a
point of discussion and possible implementationi to address the
underlying issue. But I'm referencing a constant in the Rake task so
hopefully that will help future refactors. Also, it's one reason I
needed to remove the `private_constant` declaration._
If we accept this code change, a future task is to document the ENV and
behavior.
Closes#15960
**Further considerations**:
- How might we refine this to not be as "role" reliant?
- Could we have a Site::Setting that we enable/disable
regarding the navigation links?
Regarding QA:
- Start from an empty database
- Run setup
- Verify Navigation Link exists
- Delete Navigation Link
- Run setup again
- Verify Navigation Link exists (because we don't have a user)
- Run seeds
- Delete Navigation Link
- Run setup again
- Verify Navigation Link exists (because we don't have a user)
```shell
$ cd ./path/to/forem/repo
$ rails db:drop db:create db:schema:load
$ bin/setup
$ bin/rails runner "puts NavigationLink.where(url: '/readinglist').exists?"
=> true
$ bin/rails runner "NavigationLink.where(url: '/readinglist').delete_all"
$ bin/rails runner "puts NavigationLink.where(url: '/readinglist').exists?"
=> false
$ bin/setup
$ bin/rails runner "puts NavigationLink.where(url: '/readinglist').exists?"
=> true
$ bin/rails db:seed
$ bin/rails runner "NavigationLink.where(url: '/readinglist').delete_all"
$ bin/rails runner "puts NavigationLink.where(url: '/readinglist').exists?"
=> false
$ bin/setup
$ bin/rails runner "puts NavigationLink.where(url: '/readinglist').exists?"c
=> false
```
* Bump for travis
