* Adds an Add Organization modal and e2e tests for the modal
* Conditionally redirects to the correct view upon adding org
* Combines modal tests within userIndexView.spec.js
* Removes redundant code and switches assign role and add org button positions
* Updates RSpec tests to account for conditional redirection
* Lowercases the o in organization within _user_actions_dropdown.html.erb
* Fixes failing e2e tests
* make feed image consistent with cover image
* small tweaks for the logged out view of the feed cover image`
* update feed cover image for logged in users
* update to reflect new cover image
* reverting to see if the build, builds?
* snapshot tests
* update spec to look for the right class
* too long by ONE character 121/120 🙄
* Update app/javascript/articles/components/ArticleCoverImage.jsx
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* Update app/views/articles/_single_story.html.erb
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* Update app/views/articles/_single_story.html.erb
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* no need for the anchor tag to have a bg color
* update snapshot test expectations
* remove unnecessary span with hidden class
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
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
* 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
* 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
* 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.
This PR does three things:
1. Removes a feature flag reference
2. Renames optimized to the large_forem_experimental (a value that
occurs in the code base)
3. Wraps two specs in a similar context as a later appearing spec (hence
the larger appearing change due to indentation adjustments)
Closesforem/forem#17510
* Prefer compact_blank! to delete_if blank
Rubocop suggested this. Why would I argue.
* When sort_direction is anything other than asc or desc, remove it
There is initializer code in the search/query classes that handles nil
sort_direction by adding a default value, so deleting the sort
direction is reasonable here.
* Set a default sort direction for articles of :desc
* Only sort if sort_direction and sort_by are present
* Add test case for article search with invalid parameter
* 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>
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
* Add minimal test on rss feed response
* Update date to string conversion to avoid deprecation
It wouldn't make sense to use a deprecated method here.
* feat: add the export route
* feat: base export csv
* refactor: move the svg into its own file
* feat: add the export partial
* feat: export teh correct fields etc. for the csv
* user status helper
* feat: ensure that we format the time
* feat: add a spec for the CSV
* remove space
* remove puts
* chore: remove blank space
* feat: update traits
* Update app/views/admin/users/export.csv.erb
Co-authored-by: Jamie Gaskins <jgaskins@hey.com>
* Update spec/requests/admin/users/users_export_spec.rb
Co-authored-by: Jamie Gaskins <jgaskins@hey.com>
* Update spec/requests/admin/users/users_export_spec.rb
Co-authored-by: Jamie Gaskins <jgaskins@hey.com>
* fix: export should not error for unregistered users
Co-authored-by: Jamie Gaskins <jgaskins@hey.com>
* Revert "conditionally render create post button for admins #16490 (#16606)"
This reverts commit 1cb45995cb.
* Adding conditional create post rendering
This pull request does two things:
- Reverts the forem/forem#16606
- Replaces the conditional rendering with the approach from forem/forem#17076
What to consider:
- How does this impact Cumulative Layout Shift (CLS). Prior to this
commit, we didn't "flicker" the "Create Post" button into view if the
site didn't have the conditional.
- In reverting forem/forem#16606 we remove a dependency on Turbo.
- We reduce one network call, relying instead on the async_info to carry
the visible/hidden logic.
If the "flicker" is a problem we could add a conditional in the page for
if the site has enabled that feature. However, that's a place holder as
we're looking at possible other reasons for allowing/disabling the
create a post.
This is a non-blocking PR, as in we can merge or not merge this and
proceed with what we have. Instead the main goal is to "unify" our approach.
* Update cypress/integration/seededFlows/policyFlows/limitPostCreationToAdmins.spec.js
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* Leveraging change from forem/forem#17143
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* Expect to raise error should use a block
This is kind of pedantic, since we _had_ been using a lambda, and
calling as a value. Now explicitly call the proc in a block and expect
the block to raise error.
* Replace lambda with block
Replace lambda passed as a value to an implicit block with a block.
* 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, we determined the 404 status based on the final
scoped query of the articles. That scoped query would limit to a
timeframe.
However, what we want is to not render a 404 status if we have an
"established" tag. An established tag means that we have at least one
published article (regardless of when we published).
Fixesforem/forem#16932
* Trigger Build
* WIP - use turbo to render html for create post button
todos and discussion topics
[] add tests (probably gonna be a lot of cypress 😅😭)
[] nominclature & file structure for authorization controller/routes
[] is this "authorizations" namespace approach the best?
[] authorization strategy relies on current user and caching doesn't
like `current_user`
* Use URL helper and not change the policy just yet
* put the turbo frame behind a feature flag
* Trigger Build
* Use URL helper and not change the policy just yet
* :)
* Add some test coverage and use same feature flag for consistency
* rubocop
* more rubocop
* trigger build
* adding a small sleep to see if specs pass 😩
* remove sleep
* disable Turbo session drive
* WIP
* Revert "WIP"
This reverts commit ce838672069fd703122156a0d82e5e6ad5398e0a.
* Moving spec from dashboard to root
With forem/forem#16913 the underlying test broke because we were
redirecting on the dashboard.
Co-authored-by: Jeremy Friesen <jeremy.n.friesen@gmail.com>
Prior to this commit the following situation existed:
> The path /dashboard/analytics/org/:id requires user
> authentication (e.g. signed in). However, it does not enforce
> authorization. Anyone can see this page. The page, however, uses
> javascript to populate the data. So no information, aside from the org
> name associated with the :id leaks out. The javascript API end point
> enforces organization membership.
>
> I would expect that the authorization in the HTML rendering would be
> the same as the javascript API end point.
This commit ensures that the dashboards#analytics end point uses the
same policy logic as the API analytics end points. Further, it keeps
folks who aren't org members out of the base HTML page for other orgs.
Closes forem/forem/#16985
This commit provides two things:
1. Some notes related to my analysis regarding the dashboard
2. Conditional redirects and rendering based on article policies
The code comments say most of what I want to say, but to reiterate:
When a user can't create articles nor do they already have published
articles, then we don't want to avoid showing them stats related to
articles.
Closesforem/forem#16913
Related to forem/forem#16908 and forem/forem#16931
* feat: add the feature flag route and controller
* feat: add a ff page
* feat: update to a post with a success banner
* feat: test the feature
* feat: update the styling
* feat: change feature flag to extension witha few more improvements like adding a model not backed by a table
* feat: update the text
* refactor: if you enable a FF, it gets added under the hood as well
* feat: accessibility name for label
* use keyword args
* use locales
* Update app/views/admin/extensions/index.html.erb
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* Update app/views/admin/extensions/index.html.erb
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* Update app/views/admin/extensions/index.html.erb
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* Update config/locales/controllers/admin/fr.yml
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* Update app/controllers/admin/extensions_controller.rb
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* fix: rubucop
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* Removes admin_member_view feature flags and refactors conditionals
* Removes unused admin/users/_email_tools partial and user_email_tools_spec
* Actually remove the admin/user_email_tools_spec this time
* Removes the edit route from admin.rb users
* Adds a DUS (and spec) to disable and remove admin_member_view flag
* Fixes admin/users_spec.rb failures by updating spec with Member Detail changes
* Fixes spec failures within admin_bans_or_warns_user_spec.rb
* Refactors Admin::UsersController#credit_params due to failing specs
* Removes last traces of edit_admin_user_path and fixes buttons, specs, etc.
* Use correct table heading for username
* WIP add role filter and crayonsify forms
* styling
* responsive
* Add form back in that was removed by merge conflict fix
* Suggest only valid search options
* Add basic tests for search and filter by role
* Add aria for search field
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
* Add email to aria
Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>
Co-authored-by: Paweł Ludwiczak <ludwiczakpawel@gmail.com>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com>
This commit compresses the policy check that spanned both :new? and
:enabled?. We hide the ability to upload a video (as per the altered
html.erb files of this commit) but we didn't enforce the same logic in
the controller.
Further, as Videos are a subset of Article, I'm adding the "verify the
user is not suspended" test.
Directly relates to #16483
For sleuthing this relates:
- #16728
- forem/forem#16537
- #16634
And for historical context, this relates to:
- #10955
- #10954
- forem/internalEngineering#149
As part of the [Authorization System Use Case
1:1](https://github.com/orgs/forem/projects/46) project, we are driving
towards the feature of: "Only admin's may post articles in this Forem."
This commit ensures that the API's "create an article" end-point
delivers on that feature.
Along the way, I've added reading notes and comments, to help us further
flex in the future (namely move all authorization checks into a policy
object).
Closesforem/forem#16488
* Trigger Build
* Remove cache header before_action and add ability to search by tag name
* Remove cache header before_action and add ability to search by tag name
* Add internal bulk tag endpoint to get tags by array of names or ids
* Restore V0 tags controller
* Removes logo_svg-related code
* Removes logo_svg-related specs
* Removes the logo_uploader and logo_uploader DUS spec
* Removes the logo_uploader_spec.rb
* Adds a DUS to remove the logo_svg var from the DB
* Reverts the removal of the logo_svg_uploader.rb
