What type of PR is this? (check all applicable)

• [x] Refactor
• [ ] Feature
• [x] Bug Fix
• [ ] Optimization
• [ ] Documentation Update

Description

This PR does a few things:

• Fix the client-side check for blocked content.
  • This had broken because we changed an HTML class, but were still using it for client side reference.
• Add functionality so the blocked content is hiddent on the server for the core feed.
  • At the original implementation of blocked content, this was not possible.
  • This lays the groundwork for a better overall approach to safely blocking content.
• Identified and fixed a small inefficiency in the way the home page was loaded
  • We made a useless query + sort in the home feed.

Related Tickets & Documents

https://github.com/forem/forem/issues/10744

QA Instructions, Screenshots, Recordings

There are no UI changes which should require much user testing— But you can sanity check that this works as expected.

Please replace this line with instructions on how to test your changes, as well as any relevant images for UI changes.

UI accessibility concerns?

No UI changes

Added tests?

• [x] Yes
• [ ] No, and this is why: please replace this line with details on why tests have not been included
• [ ] I need help with writing tests

Added to documentation?

• [ ] Developer Docs and/or Admin Guide
• [ ] README
• [x] No documentation needed - Fixes existing expectations

What type of PR is this? (check all applicable)

• [x] Feature
• [x] Bug Fix
• [x] Optimization

Description

This adds a new concept of busting caches for an author's cached content areas, specifically the article's sidebar "More from [author]" and bottom content "Read next" areas. The idea here is to rely more on busting the timestamp user/organization.latest_article_updated_at, and therefore can lead to less stale content (broken links, incorrect titles and tags, usernames, etc.). For now, this simply clears the cached areas when a title and tag list is updated.

I realize this won't cover all cases, but I think this is a good start, and I've also been sitting on this code for too long. Would definitely appreciate feedback here!

Related Tickets & Documents

Resolves https://github.com/forem/internalEngineering/issues/242

QA Instructions, Screenshots, Recordings

1. Enable caching in development: rails dev:cache
2. Mock the articles that will be viewed in:

• _sticky_nav: return early in get_user_stickies.rb, call method (line 4) with return author.articles.published.last(3)
• _bottom_content: replace line 210 in app/views/articles/show.html.erb with suggested_articles = (@organization || @user).articles.published.last(3)

3. Create and publish a new article via /new
4. • [ ] See that it shows up in the sidebar and bottom content areas
5. Update the created article's title and tags
6. • [ ] See that the new changes are reflected in the sidebar and bottom content areas

UI accessibility concerns?

Nope! This doesn't change any front end code.

Added tests?

• [x] I need help with writing tests. Not sure how to write a test for caching.

Added to documentation?

• [x] No documentation needed

[optional] Are there any post deployment tasks we need to perform?

We will have to monitor our alerting, and maybe turn it off for a bit. The last time we touched /articles/_sticky_nav.html.erb the alerting went off a lot, but the site didn't slow down much. cc @maestromac here.

[optional] What gif best describes this PR or how it makes you feel?

What type of PR is this? (check all applicable)

• [X] Refactor
• [ ] Feature
• [ ] Bug Fix
• [X] Optimization
• [ ] Documentation Update

Description

To make the header menu more ergonomic and accessible, this PR includes some updates to the markup and JavaScript logic. Including:

• Wrapping the logged-in header menu in an unordered list so the items are counted in a screen reader
• Adding aria-expanded to the navigation button so it indicates its state programmatically
• Opening the dropdown when you activate the button with a keyboard "click", as opposed to opening it on focus
• Sending focus to the first item when shown
• Wiring up the Escape key to close and sending focus back to the menu button

Note: the profile focus state could still use a little work, as it's not respecting border radius.

Because the navigation button also opens a logged-out overlay, I specifically did not add aria-haspopup, which is meant to indicate a sub menu. I considered adjusting the markup of that overlay to be an unordered list / menu-type-thing too, but it was going to be difficult with all of the wrapping DIVs and non-interactive items like heading and paragraph text. So I opted to leave that part alone.

A follow-up task will be to ensure this logic is repeated on all dropdown instances, starting with Storybook.

Related Tickets & Documents

Related to https://github.com/forem/forem/issues/1154

QA Instructions, Screenshots, Recordings

Navigate the header with the keyboard, using the Enter or Space key to open it and Escape to close (or tabbing past the last item, which I kept). Notice how you aren't forced through the open menu item anymore when you tab through the page.

Note: because the items are wrapped in list items now, I moved the logic to show the Forem Admin item to the LI instead of the A. This also eliminates an empty list item that was still being rendered when the Admin link was hidden for permissions reasons.

Tested in Voiceover and Safari, iOS Voiceover, Edge and NVDA, Chrome and NVDA, Firefox and NVDA.

Added tests?

• [ ] Yes
• [X] No, and this is why: We need Cypress for these tests! Alternatively I could rewrite this menu logic into an ES module and test it with Jest.
• [ ] I need help with writing tests

Added to documentation?

• [ ] Developer Docs and/or Admin Guide
• [ ] README
• [X] No documentation needed

[optional] Are there any post deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

What type of PR is this? (check all applicable)

• [ ] Refactor
• [ ] Feature
• [ ] Bug Fix
• [ ] Optimization
• [X] Documentation Update

Description

This PR updates the Dropdown Storybook stories so that they are keyboard accessible. The dropdowns can now be opened and closed by keyboard (Enter / Esc), and can also be closed by clicking outside of the dropdown.

Notes have been added to the stories to give context on accessibility implications when using the dropdown component.

This PR ensures that our Storybook documents accessible usage of the Dropdown component, however wider work is needed to ensure all dropdowns used throughout the app are consistently handling keyboard events and focus in a way optimised for accessibility. This will need to be a generalised approach that accounts for both Preact and non-Preact usage. A new issue has been created for this here: #12297

Related Tickets & Documents

Closes #11724 - Storybook dropdown should be accessible

QA Instructions, Screenshots, Recordings

• Open Storybook and navigate to Dropdowns
• In each of the Dropdown stories, you should be able to: -- Focus the activator button by pressing the Tab key -- When pressing enter on the focused button, the dropdown content should appear, and focus should be sent to the link inside the content -- When the dropdown is open, pressing Escape should close it -- When the dropdown is open, clicking outside of the dropdown content should close it -- Clicking the button with the mouse should toggle the dropdown content open/closed -- When using a screen reader (I have tested this on Safari with VoiceOver), the expanded/collapsed state of the dropdown should be announced when the activator button is focused

UI accessibility concerns?

See above QA instructions.

Please also note I've disabled a jsx-a11y warning regarding the keyUp handler on the wrapping div in both Dropdown.stories.jsx and dropdown.html.stories.jsx. The event handler has been added to the containing (non-interactive) div to ensure Escape key presses are captured to collapse the dropdown content. As the containing div isn't expected to be interactive, the linter warning is in this case a false positive. A comment has been added to the code to this effect too.

Added tests?

• [ ] Yes
• [X] No, and this is why: the underlying Dropdown component has not been changed, the code changes only impact the Storybook documentation
• [ ] I need help with writing tests

Added to documentation?

• [ ] Developer Docs and/or Admin Guide
• [ ] README
• [ ] No documentation needed

[optional] Are there any post deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

What type of PR is this? (check all applicable)

• [x] Refactor
• [x] Feature
• [ ] Bug Fix
• [ ] Optimization
• [ ] Documentation Update

Description

The issue:

Occasionally, users are unable to delete their own DEV accounts. Sometimes the link provided to them will lead to a 404.

The solution:

This PR improves the self-deletion error messaging in Users::Controller#confirm_destroy to mitigate confusion when self-deleting an account.

To accomplish this, this PR does the following:

• Reworks the code within #confirm_destroy to be more consistent with the other destroy-related (#request_destroy and #full_delete) methods by mirroring how the error message is set and displayed (flash[:settings_notice]).
• Adds confirm_destroy to the exception in the verify_authorized! after_action.
• Adds a conditional to #confirm_destroy that sets the current_user and does one of two things: authorizes the user or alerts the user that, "You must log in to proceed with account deletion." and redirects the user to the "Sign In" page.
• Adds a conditional that checks whether the destroy_token is blank or not. If the destroy_token is blank, then a message is displayed, alerting the user that their token has expired. Otherwise, it raises an error due to a destroy_token mismatch.
• Adds tests around all edge cases to ensure that the proper error messages are raised under the proper circumstances.

Related Tickets & Documents

Closes issue 374

QA Instructions, Screenshots, Recordings

⚠️ Heads-up: caching is not enabled by default in development. In order to properly QA these changes, you must first enable caching. To enable and disable caching in development, run the following command: Rails dev:cache.

🥇 Pro-tip: to make QAing these changes easier, add a delimiter to your console to easily search for the email containing the token and/or have development.log open so that you can search for the email containing the token by. You can search for the delimiter that you added (I use ------ or ~~~~~~) or the word mime to locate the email containing the token once you have clicked the "Delete Account" button (see QA instructions below for further context!).

To test the self-deletion flow as a logged-in user with a valid token, please do the following:

• Log in and navigate to /settings/account. Click the "Delete Account" button found at the bottom of the page:

• You should then be redirected back to settings/account where you should see a message confirming that you've requested an account deletion:

• Copy the link containing the destroy_token from your logs and paste it into your browser. You should be taken to the "Destroy your account" page as expected. 🛑 If you choose to destroy your account at this time, you will need to log in as a different user to QA the rest of this PR:

Before QAing the invalid token flow, please read the following heads-up:

⚠️ Heads-up: In order to test the invalid token edge case, please delete the key in a console like so (you will need to know the user's ID in order to delete the key!):

[2] pry(main)> Rails.cache.delete("user-destroy-token-11")
Cache delete: user-destroy-token-11
=> 1

I have also been setting destroy_token within UsersController#confirm_destroy equal to an empty string only for testing purposes--this mimics a .blank token:

  def confirm_destroy
    destroy_token = ""

To test the self-deletion flow as a logged-in user with an invalid token, please do the following:

• Log in and navigate to /settings/account. Click the "Delete Account" button found at the bottom of the page:

• You should then be redirected back to /settings/account where you should see a message confirming that you've requested an account deletion:

• Copy the link containing the destroy_token from your logs and paste it into your browser. Rather than being taken to the "Destroy your account" page, a flash message should appear, stating: "Your token has expired, please request a new one. Tokens only last for 12 hours after account deletion is initiated." You should be redirected back to /settings/account:

⚠️ Heads-up: In order to test the invalid token edge case, please delete the key in a console like so (you will need to know the user's ID in order to delete the key!):

[2] pry(main)> Rails.cache.delete("user-destroy-token-11")
Cache delete: user-destroy-token-11
=> 1

I have also been setting destroy_token within UsersController#confirm_destroy equal to an empty string only for testing purposes--this mimics a .blank token:

  def confirm_destroy
    destroy_token = "mismatched token"

To test the self-deletion flow as a logged-in user with a mismatched token, please do the following:

• Log in and navigate to /settings/account. Click the "Delete Account" button found at the bottom of the page:

• You should then be redirected back to /settings/account where you should see a message confirming that you've requested an account deletion:

• Copy the link containing the destroy_token from your logs and paste it into your browser. Rather than being taken to the "Destroy your account" page, you should hit a 404 and an ActionController::RoutingError, "Not Found" error should be raised in your console:

To test the self-deletion flow as a logged-out user, please do the following:

• In an incognito window, copy the link containing the destroy_token from your logs and paste it into your browser. Rather than being taken to the "Destroy your account" page, you should be redirected to the "Sign In" page and a flash message should appear, stating: "You must be logged in to proceed with account deletion.":

UI accessibility concerns?

N/A

Added tests?

• [x] Yes
• [ ] No, and this is why: please replace this line with details on why tests have not been included
• [ ] I need help with writing tests

Added to documentation?

• [ ] Developer Docs and/or Admin Guide
• [ ] README
• [x] No documentation needed

[optional] Are there any post-deployment tasks we need to perform?

N/A

[optional] What gif best describes this PR or how it makes you feel?

What type of PR is this? (check all applicable)

• [x] Bug Fix

Description

We had a bug where we cached the custom rewarding message for the notification a user receives when they are awarded a new badge. This fixes it by removing the cache altogether, since it wasn't really preventing any N+1 calls.

Related Tickets & Documents

Closes #11957 and closes #11646

QA Instructions, Screenshots, Recordings

1. Paste this into rails console:

# feel free to replace User.last with yourself
ba = BadgeAchievement.create!(badge_id: 1, rewarding_context_markdown_message: "test", user: User.last)
Notifications::NewBadgeAchievement::Send.call(ba)

2. Visit localhost:3000/notifications, you can add ?username=User.last.username if you ended up using the last user instead of yourself. You'll need to be a super_admin.

UI accessibility concerns?

Nope

Added tests?

• [x] No, and this is why: don't think I can test the cache

Added to documentation?

• [x] No documentation needed

[optional] Are there any post deployment tasks we need to perform?

Nope, the data update script should take care of things.

[optional] What gif best describes this PR or how it makes you feel?

What type of PR is this? (check all applicable)

• [ ] Refactor
• [X] Feature
• [ ] Bug Fix
• [ ] Optimization
• [ ] Documentation Update

Description

Adds a permalink icon/link to each of the headings in /p/editor_guide. Linked icon is hidden until a user hovers over the heading, or tabs to it with the keyboard.

Related Tickets & Documents

Closes #4507 - Feature request: Add linkable "id" to each Editor Guide section headers

QA Instructions, Screenshots, Recordings

• When a user hovers over a heading on the /p/editor_guide page, a link icon appears
• Icon links to the heading's anchor ID & clicking it takes you to that location on the page
• Link icons are also shown when a user tabs through the content of the page with the keyboard, becoming visible on focus, and disappearing when unfocused again
• On mobile devices the link icon becomes visible if the heading is tapped on (since there's no hover)
• The new permalinks are surfaced to screen reader users as "permalink for { heading name }"

I've tested this on Firefox/Safari/Chrome, and chrome on android mobile.

UI accessibility concerns?

I've given the new links meaningful labels that clearly distinguish them as 'permalinks' so it's clear they relate to the inner doc sections. The links although hidden, are surfaced to screen reader users and become visible with default focus outline when tabbing with the keyboard.

Added tests?

• [ ] Yes
• [X] No, and this is why: code is all static and there's no logic to verify here
• [ ] I need help with writing tests

Added to documentation?

• [ ] Developer Docs and/or Admin Guide
• [ ] README
• [X] No documentation needed

[optional] Are there any post deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

What type of PR is this? (check all applicable)

• [X] Bug Fix

Description

#12131 described a workflow that led to the accidental publishing of a draft post after fixing validation errors. This PR fixes that.

Related Tickets & Documents

#12131

QA Instructions, Screenshots, Recordings

Follow the steps as outlined in the original issue and ensure that you still have the option to save a draft at the last step, not only "Save changes".

• Create a new blog post
• Add something that is validated, for example: {% codepen https://codepen.io/DonKarlssonSan/pen/dyMaRJN default-tab=result %}
• "Save draft"
• Edit the blog post
• Change the content to something that is not valid, for example: {% codepen https://codepen.io/DonKarlssonSan/pen/dyMaRJN default-tab="result" %}
• Click "Save draft"
• An error message is displayed: "Whoops, something went wrong: base: Invalid Options"
• Fix the error (remove the "" around result)
• There should be both "Publish" and "Save Draft" buttons

UI accessibility concerns?

None

Added tests?

• [X] No, and this is why: verifying this approach first, then figuring out how to write a regression test for this in JS.

Added to documentation?

• [X] No documentation needed

Is your feature request related to a problem? Please describe.

Related to this PR, this issue is to create a UX flow that alerts forem admins when they enable an Authentication Provider, but have not entered the keys for that provider. Right now, there is no indication to the admin that an authentication provider's keys are missing.

Describe the solution you'd like

The goal here is to let the admin know that they need to fill out required form fields for their action to successfully occur.

• When user clicks "Enable" to setup new method, don't show any warning notice.
• They either don't complete the form, or enter information unsuccessfully.
• They attempt to confirm the update with inputting the confirmation text and clicking "Update configuration"
• Trigger a modal with messaging akin to what is below:

Describe alternatives you've considered

In the original PR, a warning banner was displayed when the authentication provider's keys were missing (see screenshot below). However, Team Design proposed a different approach that would not only be a better UX, but will fit into the larger strategy for the set of improvements we'll make with Admin in general.

What type of PR is this? (check all applicable)

• [ ] Refactor
• [ ] Feature
• [x] Bug Fix
• [ ] Optimization
• [ ] Documentation Update

Description

All our auth services have shown to occasionally fails authentication because they can pass a profile which has no remote image string.

This creates a Users::SafeRemoteProfileImageUrl which accepts a URL if passed, but falls back to our default generator if not.

It does not attempt to fully download and validate the image which I think would be redundant with the duties of carrierwave, and is plenty to fix this particular case without doing the extra work of downloading the image twice.

Related Tickets & Documents

Closes https://github.com/forem/internalEngineering/issues/278

QA Instructions, Screenshots, Recordings

A manual QA approach is described in the issue.

UI accessibility concerns?

No

Added tests?

• [x] Yes
• [ ] No, and this is why: please replace this line with details on why tests have not been included
• [ ] I need help with writing tests

</div>
<div class="gh-btn-container"><a class="gh-btn" href="https://github.com/forem/forem/pull/12390">View on GitHub</a></div>