Welcome back to another Repo Recap, where we (usually) cover last week's contributions to dev.to's repository and the iOS repo. This edition will be covering three weeks, March 16 to April 5.
Features
- Night mode is officially out in public beta, as well as changing the font of posts! You can change it in your settings: https://dev.to/settings/misc Thanks to @ben for getting it out the door!
Add proper css vars to user style config
#2105
What type of PR is this? (check all applicable)
- [ ] Refactor
- [x] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
This renders the proper night theme using CSS variables. This currently takes hardcoded theme info right in application.html.erb
but could be modified to take variables in the future.
Also added another font option: comic sans
.
- Thanks to @picocreator making our Docker build run in a one line step! More details in the PR:
Feature : docker-run.sh script + docker container build
#1844
What type of PR is this? (check all applicable)
- [ ] Refactor
- [x] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
A single bash script that helps quickly setup either a DEV or DEMO environment
bash-3.2$ ./docker-run.sh
#---
#
# This script will perform the following steps ...
#
# 1) Stop and remove any docker container with the name 'dev-to-postgres' and 'dev-to'
# 2) Reset any storage directories if RUN_MODE starts with 'RESET-'
# 3) Build the dev.to docker image, with the name of 'dev-to:dev' or 'dev-to:demo'
# 4) Deploy the postgres container, mounting '_docker-storage/postgres' with the name 'dev-to-postgres'
# 5) Deploy the dev-to container, with the name of 'dev-to-app', and sets up its port to 3000
#
# To run this script properly, execute with the following (inside the dev.to repository folder)...
# './docker-run.sh [RUN_MODE] [Additional docker envrionment arguments]'
#
# Alternatively to run this script in 'interactive mode' simply run
# './docker-run.sh INTERACTIVE-DEMO'
#
#---
#---
#
# RUN_MODE can either be the following
#
# - 'DEV' : Start up the container into bash, with a quick start guide
# - 'DEMO' : Start up the container, and run dev.to (requries ALGOLIA environment variables)
# - 'RESET-DEV' : Resets postgresql and upload data directory for a clean deployment, before running as DEV mode
# - 'RESET-DEMO' : Resets postgresql and upload data directory for a clean deployment, before running as DEMO mode
# - 'INTERACTIVE-DEMO' : Runs this script in 'interactive' mode to setup the 'DEMO'
#
# So for example to run a development container in bash its simply
# './docker-run.sh DEV'
#
# To run a simple demo, with some dummy data (replace <?> with the actual keys)
# './docker-run.sh DEMO -e ALGOLIASEARCH_APPLICATION_ID=<?> -e ALGOLIASEARCH_SEARCH_ONLY_KEY=<?> -e ALGOLIASEARCH_API_KEY=<?>'
#
# Finally to run a working demo, you will need to provide either...
# './docker-run.sh .... -e GITHUB_KEY=<?> -e GITHUB_SECRET=<?> -e GITHUB_TOKEN=<?>
#
# And / Or ...
# './docker-run.sh .... -e TWITTER_ACCESS_TOKEN=<?> -e TWITTER_ACCESS_TOKEN_SECRET=<?> -e TWITTER_KEY=<?> -e TWITTER_SECRET=<?>
#
# Note that all of this can also be configured via ENVIRONMENT variables prior to running the script
#
#---
And does the deployment using docker. Includes option to do a reset prior to deployment.
Optional contextual information provided here : https://dev.to/uilicious/adopt-your-own-devto----with-a-single-command-almost-1c04
Need advice on ...
if someone can guide me on how to run dev.to in "Production" mode, it would be great in improving the overall docker container performance
Added to documentation?
- sample readme for docker hub : https://github.com/uilicious/dev.to-docker/blob/feature/docker-run-script/dockerhub-readme.md
- https://cloud.docker.com/u/uilicious/repository/docker/uilicious/dev.to
- I can modify readme if this looks good?
What gif best describes this PR
What gif best describes how it makes you feel?
- You can now add prefilled markdown via parameters when writing a new post.
https://dev.to/new
! More details on this feature in the future! PR by @ben
Add arbitrary prefilled markdown for /new
#2108
What type of PR is this? (check all applicable)
- [ ] Refactor
- [x] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
This feature allows users to supply a url like dev.to/new?prefill=A BUNCH OF MARKDOWN
in order to supply arbitrary templates. This is similar to the tag prefilled templates.
- We now have some native view tracking for your posts. This helps us display trends a bit easier. PR by @ben
Add organic page view sums and modify seo boostables
#2109
What type of PR is this? (check all applicable)
- [ ] Refactor
- [x] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
This allows us to store data about organic page views (recent and all time) and display trends this way. For use on side bar to help maintain SEO to pages already doing well with Google.
- Thanks to @willamesoares, the comment box/textarea is now resizable! Enlarge to your heart's content.
Allow users to resize edit comment textarea element
#1866
What type of PR is this? (check all applicable)
- [ ] Refactor
- [x] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Based on complaints, this allows users to resize the text area element while editing a comment.
Related Tickets & Documents
#1554
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- @davefollett Tag removal textarea changes: replaced placeholder text with improved
Tag removal textarea changes: replaced placeholder text with improved …
#2063
What type of PR is this? (check all applicable)
- [ ] Refactor
- [x] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Tag removal textarea changes:
-
Replaced placeholder text with improved sample textImproved placeholder text to be clear that moderators only need to provide a reason. - Increased height so all text shows by default
Related Tickets & Documents
Resolves #2001
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- @ben Add themes to main list, notifications, and chat
Add themes to main list, notifications, and chat
#2119
What type of PR is this? (check all applicable)
- [ ] Refactor
- [x] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
More theme goodness!
- A small feature, but we now set the site logo via an environment variable. PR by @ben
Add ability to set site logo via ENV variable
#2123
What type of PR is this? (check all applicable)
- [ ] Refactor
- [x] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
This allows admins to set new logos for special events as well as generalized instances of the platform.
- Thanks to @rhymes for adding clearer error messaging when an image upload fails!
Handle images uploading errors
#2018
What type of PR is this? (check all applicable)
- [ ] Refactor
- [x] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
Currently, when an image upload results in an error, the server "breaks", a HTTP 500 unhandled error is generated and the user is shown a generic "invalid file" for the comment editor and editor v1 and nothing at all for editor v2.
In this PR I added/changed the following:
-
a proper error handling server side (an image upload error shouldn't result in HTTP 500, because the error is fine, it's the client that sent something wrong)
-
errors that are not client dependent should indeed generate HTTP 500
-
messages returned by CarrierWave that are related to client errors are directly shown to the user (I checked the from CarrierWave's own locale/en.yml and they seem ok to be consumed by the user
-
messages returned by CarrierWave that are related to server errors are still handled but a generic "a server error has occurred" is sent to the client (should these by sent to a bug tracker as well?)
-
I added error handling to the client side
A note: I'm not super confident in the way I chose to display the upload error in the editor v2, let me know if it should be displayed in a different way or if it should have a better styling.
I added screen recordings for the three cases
Related Tickets & Documents
Closes #936, closes #2008
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Editor v1
Editor v2
Comment editor
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- @dabrorius added a list of followed organizations to dashboard. Thanks, @dabrorius!
Add a list of followed organizations to dashboard
#2157
What type of PR is this? (check all applicable)
- [ ] Refactor
- [x] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Add a list of followed organizations to user's dashboard.
Related Tickets & Documents
#1958
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
[optional] What gif best describes this PR or how it makes you feel?
- We now have some shop banners on the left side, in case you want to DEV up your style. PR by @ben
Add areas for DEV Shop displays on left sidebar
#2162
What type of PR is this? (check all applicable)
- [ ] Refactor
- [x] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Adds ability to put shop and other theme day content at bottom of left sidebar
- Thanks to @jess, we can now merge accounts via our internal tooling! If you are having an issue with a duplicate account, feel free to email yo@dev.to.
Merge user and give pro users trusted role
#2176
What type of PR is this? (check all applicable)
- [ ] Refactor
- [x] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
- Automatically give "PRO" users trusted role. I'm sure PRO stuff will be moved out of this area soon but I think this makes sense for now.
- Add merge user functionality to /internal.
- Prevent merge if account to be deleted has two identities (likely indicator that this account shouldn't be deleted)
- @ben updated our onboarding with several new recommended tags. Check it out below:
Add several recommended tags for onboarding
#2217
What type of PR is this? (check all applicable)
- [ ] Refactor
- [x] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
This adds some popular tags to the onboarding flow.
- Thanks to @mariocsee, you can now block or easily report abuse in Connect chat!
block/report abuse in dev-connect
#2074
What type of PR is this? (check all applicable)
- [x] Feature
Description
Adds blocking and report abuse to DEV Connect to prevent abuse. Users can keep their inbox open and blocked users cannot re-open chat with them.
blocked
is now an option for ChatChannel
status
. This prevents a ChatChannel
from being rendered in /connect
and doesn't allow status
to be changed to active
. Currently, there is no way to unblock someone on /connect
on the user side.
Related Tickets & Documents
continues off from #1563
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Blocking
Reporting Abuse
Added to documentation?
- [x] no documentation needed
- For April Fool's Day, we tried to do something but it ended up going a bit haywire... Here's the offending PR, and the related post:
Make comic sans default
#2257
What type of PR is this? (check all applicable)
- [ ] Refactor
- [x] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Making good choices
- We now have API endpoints for analytics! This is currently only available to pro members (more details on that to come).
Add API Analytics Endpoints
#2293
What type of PR is this? (check all applicable)
- [x] Feature
Description
This adds analytics endpoints via API for stats! This is currently only available to pro members. Pro members who are also a part of an organization can view their respective organization's stats as well via API.
There are three endpoints currently:
-
/api/analytics/totals
, which gives you a summary of all your posts' stats, -
/api/analytics/past_day
which gives you the past 24 hours worth of data, and -
/api/analytics/historical
, which gives you a snapshot of data of each day based on a certain timeframe.
You'll need to generate an API token via https://dev.to/settings/account to authorize yourself. If you'd like view your organization's data, you can add the organization_id
parameter. Here's an example endpoint: https://dev.to/api/analytics/totals?api_token=blahblahblah&organization_id=9001
For /historical
, you must add at least a starting date parameter start
. The ending date parameter end
is optional, and leaving it out will give you all data up to the current date. Both date parameters must be in the format of yyyy-mm-dd
(ex. 2019-03-29) Example endpoints:
https://dev.to/api/analytics/historical?api_token=blahblahblah&start=2019-03-26
https://dev.to/api/analytics/historical?api_token=blahblahblah&start=2019-03-01&end=2019-03-14
Added to documentation?
Not yet, but will add to docs.dev.to soon!
[optional] What gif best describes this PR or how it makes you feel?
- We also have a dashboard for more detailed analytics for said pro members:
Enable Org Pro Dashboard View
#2060
This PR enables the org pro dashboard view. I decided to leave refactoring and making more data available for the next PR.
- Trusted members of the community can now suggest social copy for posts! PR by @ben:
Add ability for trusted users to suggest social copy for posts
#2306
What type of PR is this? (check all applicable)
- [ ] Refactor
- [x] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
This feature allows community members to offer Twitter copy for posts that will go out to social accounts. This is the minimum viable version. May be rough around the edges.
-
@bolariinwa added support for the HTML
<mark>
element in Markdown. This means you can now highlight things as you'd like. Thanks, @bolariinwa!
Add support for mark element in markdown
#2089
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [X] Bug Fix
- [ ] Documentation Update
Description
The <mark>
is useful for authors to highlight important text. Following the suggestion made by @link2twenty, I updated the tags
list in rendered_markdown_scrubber.rb
.
Related Tickets & Documents
The issue number is #1870.
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [X] no documentation needed
- https://dev.to/videos is out! It's our landing page for all natively uploaded videos on dev.to. We'll be making more improvements soon. PR by @jess:
"/videos"
#2291
What type of PR is this? (check all applicable)
- [ ] Refactor
- [x] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Adds a page (/videos) to see all posts w. native video content.
Bug Fixes / Other Contributions
- Thanks to @jess for adding Scout config and gem in! I was personally excited to see some preliminary data in the email we received.
add scout config and gem
#2219
What type of PR is this? (check all applicable)
- [ ] Refactor
- [x] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
- Adds scout gem and config file
- Thanks to @ben for updating our Airbrake (error monitoring service we use) to ignore some extraneous errors, like
ActiveRecord::RecordNotFound
.
Ignore unnecessary airbrake notices
#2283
What type of PR is this? (check all applicable)
- [ ] Refactor
- [x] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
We don't need to log every recordnotfound, as a lot are natural from articles that are unpublished. They represent a small number of overall requests but a large number of our error tracking. This cleans up some of the mess.
Also adds a filter to randomly not track all JS errors. We're okay to only track some and still get a pretty accurate look in general IMO. We can revisit later.
- @ben resolved an issue where crossposted posts were improperly sorted because of their original posting date. Thanks, Ben!
Add originally_published_at for more accurate crossposted info
#2076
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
Currently we set published_at
when a post comes in crossposted but the logic is a little messed up in terms of where it fits in the feed. This is not the whole fix, but this is a new column to track the original date more accurately and manually fix a few bad ones.
- @perigk updated our "Additional Tech" docs to mention we use Git and GitHub for version control and issue tracking. Thanks, @perigk!
Updated addl-tech.md
#2078
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [ ] Bug Fix
- [X ] Documentation Update
Description
Update addl-tech, with two kinda-pedanctic options.
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ x] docs.dev.to
- [ ] readme
- [ ] no documentation needed
[optional] What gif best describes this PR or how it makes you feel?
- @ben fixed a possible race condition with our native page view tracking. Details below:
Fix possible race condition with page view tracking
#2103
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
When a page view is tracked, we later update that view to track that the user was still on the page.
But sometimes there is an issue creating that first page view. This PR helps account for that scenario.
- Thanks to @bolariinwa for adding links to tags everywhere they appear. The missing place was in the dashboard.
Add link for tags everywhere they appear
#2088
What type of PR is this? (check all applicable)
- [ ] Refactor
- [X] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
To keep the expectation of tags being clickable, I updated _dashboard_article.html.erb so tags on posts on the dashboard are wrapped in a link tag and link to their respective pages.
Related Tickets & Documents
The issue number is #1991
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [X] no documentation needed
- @lightalloy fixed a bug where a reaction notification had leftover data from removed reactions. Thanks, Anna!
Fix reaction notification json data when a reaction is destroyed
#2112
What type of PR is this? (check all applicable)
- [x] Bug Fix
Description
Keep only persisted data in the notifications json_data
when a destroyed reaction record is passed to the Notification#send_reaction_notification
Related Tickets & Documents
#2111
- @keshavbiswa Enabled two Rubocop cops. Thanks, @keshavbiswa!
Enabled Rubocop/Naming Cop
#2115
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Enabled Naming/AsciiIdentifier Cop and Naming/FileName Cop
Related Tickets & Documents
#2021
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
[optional] What gif best describes this PR or how it makes you feel?
-
@selbekk fixed a bug by properly making words plural when needed with one of my favorite Rails methods:
pluralize
. Thanks, @selbekk!
Fix bug where singular values get plural endings
#2069
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
I've added two ternaries to add a trailing s to "comment" and "reaction" if and only if the count is not 1.
Related Tickets & Documents
#2067
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
[optional] What gif best describes this PR or how it makes you feel?
- @edisonywh fixed an issue where going to a badge that does not exist would result in a server error instead of a 404 page. Thanks, @edisonywh!
Redirect to 404 if badge not found
#2066
Previouly we do not check if @badge
exists before redirecting to
:show
, this commit fixes that by redirecting to 404.
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
Related Tickets & Documents
#2051
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
[optional] What gif best describes this PR or how it makes you feel?
-
@rhymes fixed a longstanding issue where a Front Matter
description
was not escaping properly. Thanks, @rhymes!
Fix unescaped front matter description in MarkdownFixer
#1989
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
I set out to fix https://github.com/thepracticaldev/dev.to/issues/96 - the initial idea was to create more meaningful error messages, I then realized the front matter is parsed by a third party library, so I abandonded the idea.
I decided to just fix the bug created by unwrapped chars in the description (which triggered the issue in the first place)
Related Tickets & Documents
Closes #96
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Before
After
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
-
@rhymes updated our error raising to properly reflect the correct error
ActiveRecord::NotFound
instead ofActionController::RoutingError
. Thanks again, @rhymes!
Change not found 404 raised exception in ApplicationController
#2118
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
After the discussion we had around #2066 and this comment by @lightalloy, I'm submitting this PR to change the default exception raised by the not_found
method.
FYI Rails renders the 404 page with both ActionController::RoutingError
and ActiveRecord::ActionNotFound
so there's no change in functionality. I'm not even sure the exception raised makes the code clearer because it's transparent to the pattern:
Model.find_by_(id) || not_found
it just aligns to what the finder methods with the !
raise.
This aforamentioned pattern appears 15 times in the code, instead the Rails default:
Model.find_by!(id)
appears only 3 times, all in the tags_controller.rb
file.
Related Tickets & Documents
#2066
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- @lightalloy has been refactoring and moving notifications into separate jobs. This PR moved the new follower notification. Thanks, Anna!
Moved sending new follower notification to a separate job
#2030
What type of PR is this? (check all applicable)
- [x] Refactor
Description
- created a separate Active Job to handle new follower notification
- pass attributes instead of a
Follow
instance to the job to get rid of deserialization error. (in this case, we can't just passfollow_id
, cause afollow
might be destroyed at that moment) - moved the new follower notification logic to a service object
- kept the old
Notification
interface (send_new_follower_notification
andsend_new_follower_notification_without_delay
)
Related Tickets & Documents
#1996
-
@keshavbiswa enabled the
Rubocop/Rails
cops. Thanks, @keshavbiswa!
Enabled Rubocop/Rails cops
#2125
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Enabled Rubocop/Rails cops
Related Tickets & Documents
#2021
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- @glennmen refactored how we award the yearly badges to a way that is much more flexible than our previous implementation. Thanks, @glennmen!
Refactor award_yearly_club_badges
#2035
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Refactor award_yearly_club_badges
so that adding new yearly badges will be easier and require less changes.
I got a couple of questions about the refactor:
-
Do you want it in a loop or more like
award_streak_badge(num_weeks)
? I think the for loop is a better option, you will only need to increase a variable and not add a new line tofetch.rake
https://github.com/thepracticaldev/dev.to/blob/0b84ae9acba3658af1112aeb520a049e852ffd27/app/labor/badge_rewarder.rb#L47 https://github.com/thepracticaldev/dev.to/blob/0b84ae9acba3658af1112aeb520a049e852ffd27/lib/tasks/fetch.rake#L73-L75 -
What do you guys think is the best way to set the parameter? I currently added it as a variable in the function.
-
Will changing the slug to a numeric number value instead of the word value (
one-year-club
->1-year-club
break existing badges? How to best handle this in your opinion?
Any other feedback is welcome!
Related Tickets & Documents
https://dev.to/ben/comment/8inc
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
[optional] What gif best describes this PR or how it makes you feel?
-
@venarius made some UX improvements by updating the
editor_version
field to a<select>
field. It was originally a<text>
field. Thanks, @venarius!
UX: Changed editor_version to a select field
#2107
What type of PR is this? (check all applicable)
- [ x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Changed the editor_version
input is settings/misc
to a select
field
Related Tickets & Documents
Issue #1775
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [ x] no documentation needed
This is also my first PR to dev.to
Enabled rubocop/lint
#2130
What type of PR is this? (check all applicable)
- [ x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
This enables the rubocop/lint
Related Tickets & Documents
https://github.com/thepracticaldev/dev.to/issues/2021
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [ x] no documentation needed
Update Faker and fix deprecated Faker::Overwatch use
#2142
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Update Faker::Overwatch.quote
and fix deprecation notices in tests.
NOTE: Faker::Overwatch.quote is deprecated; use Faker::Games::Overwatch.quote instead. It will be removed on or after 2019-01-01. Faker::Overwatch.quote called from /home/travis/build/abraham/dev.to/spec/factories/badges.rb:7.
Related Tickets & Documents
n/a
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
n/a
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
Add alternative text for organization logo
#2148
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
Organizations logos are missing the alternative texts in a few places.
Thanks to Lighthouse for catching this on the homepage.
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
-
@rhymes replaced all the Ruby
put
with proper logging. Thanks for that!
Replace puts with proper logging
#2139
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
While examining another PR I noticed there was a puts
in the wild in an error handler, I decided to use proper logging instead, ended up replacing all puts
.
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- @link2twenty fixed an "off by one" CSS issue with the sidebar profile's width. Thanks, @link2twenty!
Fix sidebar-profile-snapshot width
#2145
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
The 3 hardest problems programmers face are
- Naming things
- Off by 1 errors
.sidebar-profile-snapshot
width was wider than the rest of the sidebar by 1px.
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
Switch fix-db-schema-conflicts to published version
#2161
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Switch from fork to published version.
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
-
@link2twenty made several changes for night mode. Thanks, @link2twenty!
@cyrillefr enabled performance related Rubocop cops. Thanks, @cyrillefr!
Enable performance rubocop cops
#2140
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Enabled all Performance cops. Refactored where needed.
Related Tickets & Documents
https://github.com/thepracticaldev/dev.to/issues/2021
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
[optional] What gif best describes this PR or how it makes you feel?
Remove klipse script unused partial
#2164
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
While working on #2153 I noticed this partial isn't used anywhere
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- @rhymes made a security improvement and added SRI (subresource integrity) to script tags where applicable. Thanks, @rhymes!
Add SRI to script tags where applicable
#2153
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
Security wise all external scripts should have SRI enabled (aka subresource integrity). This allows the browser to check if the external source has been tampered with (and thus not load the script).
I've only added to those <script>
sites that had the information publicly available and that pointed to a specific version of the resource.
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- @mscccc improved how we render our social cards by using HTML/CSS to Image. Check out the PR for the details. Thanks, @mscccc!
Social Card upgrade - Part 1
#2090
Dev.to + HTML/CSS to Image ⚡️
This gives the SocialPreviewsController
the ability to render png
's.
Current URL:
https://res.cloudinary.com/practicaldev/image/url2png/s--pncSBGoq--/c_fill,g_north,h_400,w_800/https://dev.to/social_previews/article/75561%3Fbust%3D0-Build%20a%20GitHub%20Action%20with%20Ruby-true"
Future:
https://dev.to/social_previews/article/89948.png
Fancy Gif
Backwards compatible
Implemented this so that this PR is a no-risk deploy. Nothing is breaking. All the old URLs work exactly the same as before. The social_preview routes now also support .png
.
How does caching work?
The rendered html is used as the cache key (Dalli SHAs it). This means, we get cache busting "for free", the cache will bust whenever the template changes, or the data included in the template (such as article title or user's name) updates.
To do this, I had to remove some of the randomness in the templates. They are now "deterministically random", meaning the object's ID is used as the seed. So you get the same sequence of random numbers each time.
Release Plan
- [ ] Set
HCTI_API_USER_ID
andHCTI_API_KEY
env variables. - [ ] Ship it
- [ ] Check some preview urls, make sure they look
✨ - [ ] Follow up PR to update meta tags to point at new URLs
What type of PR is this? (check all applicable)
- [X] Feature
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
(https://hcti.io/v1/image/e632881d-b0f6-4d33-be3f-a5863cf260a2)
(https://hcti.io/v1/image/7455df4e-636e-4bed-b0f6-90cb419977fb)
(https://hcti.io/v1/image/6c52de9d-4d37-4008-80f8-67155589e1a1)
Added to documentation?
- [x] no documentation needed
Add social.coop
#2170
Closes #2169
Add the https://social.coop Mastadon instance to trusted list.
Bug fix broken giphy url
#2180
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
Fixes incorrect giphy url so that gif gets loaded correctly.
Related Tickets & Documents
Fixes #2179
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
-
@abraham enabled many
Rubocop/Rails
cops, and resolved some of the issues along the way. Thanks, @abraham!
Enables Rails cops
#2186
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
The Ruby on Rails Rubocop rules are disabled by default. This enables most of them.
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
[optional] What gif best describes this PR or how it makes you feel?
- @abraham simplified our development setup a bit. Check out the PR for the details. Thanks again, @abraham!
Simplify development setup
#2143
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [x] Documentation Update
Description
Reduces the steps for setting up development (11 steps mac/linux and 13 steps windows to 7 steps for mac/linux) by using convention of the default database config, removing manual steps that bin/setup
handles, and adding foremen
to development gems group.
Related Tickets & Documents
https://github.com/thepracticaldev/dev.to/pull/1162
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
n/a
Added to documentation?
- [x] docs.dev.to
- [x] readme
- [ ] no documentation needed
[optional] What gif best describes this PR or how it makes you feel?
- @lightalloy moved the new reaction notification method to a separate job. Thanks, Anna!
Moved new reaction notification method to a separate ActiveJob
#2122
What type of PR is this? (check all applicable)
- [x] Refactor
- [x] Bug Fix
Description
- created a separate Active Job to handle new reaction notification
- pass attributes instead of a Reaction and receiver instances to the job to avoid deserialization errors
- moved the new reaction notification logic to a service object
- kept the old
Notification
interface (methodssend_resend_reaction_notification
,send_reaction_notification_without_delay
)
Related Tickets & Documents
#1996 #1621
-
@dabrorius removed the unused
/dashboard/following_users
route. Thanks, @dabrorius!
Remove `dashboard/following_users` route
#2191
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
The dashboard/following_users
route has been deprecated a while ago and replaced with dashboard/following
route.
This commit removes the old route and updates the code to reflect it.
Related Tickets & Documents
This was discussed briefly with @benhalpern here: https://github.com/thepracticaldev/dev.to/pull/2157#issuecomment-475366986
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
No UI changes
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
[optional] What gif best describes this PR or how it makes you feel?
- @jess updated the README. Thanks, Jess!
Update Readme
#2206
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [ ] Bug Fix
- [x] Documentation Update
Description
- remove product roadmap since we never got in that habit
- ask for design mockups for frontend changes
- update mario and anna to list of contributors
- @aspittel fixed some night mode issues with the pro dashboard. Thanks, Ali!
Feature/pro dashboard
#2178
What type of PR is this? (check all applicable)
- [ ] Refactor
- [X] Feature
- [X] Bug Fix
- [ ] Documentation Update
Description
Fixes dark theme issues with the pro dashboard (though can still get further cleaned up) and also removes the recent reactors at the bottom.
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- @lightalloy DRY'ed up some of the Active Jobs tests. Thansk, Anna!
DRY the Active Jobs specs
#2215
What type of PR is this? (check all applicable)
- [x] Refactor
Description
Created a shared example to test that the ActiveJobs are enqueued when #perform_later
is called
- @ben cleaned up some new chat channel functionality and added tests. Thanks, Ben!
Clean up new chat channel functionality and add tests
#2220
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Refactor and add tests for chat channel functionality. Kept the general code arrangement about the same.
Improved article slug generation
#2218
What type of PR is this? (check all applicable)
- [ ] Refactor
- [X] Feature
- [X] Bug Fix
- [ ] Documentation Update
Description
Ruby on Rails has a built-in slug generator since Rails 2.2 https://www.rubydoc.info/github/rails/rails/String:parameterize
This generator fix the slug generation when non english characters are used on title improving SEO. For example: https://dev.to/lito_ordes/redimensionando-imgenes-en-tiempo-de-ejecucin-con-packer-57c6
- @lightalloy enabled a couple of Rubocop cops which had no violations. Thanks, Anna!
Enabled a couple of rubocop cops which have no violations
#2214
What type of PR is this? (check all applicable)
- [x] Refactor
Description
Enabled a couple of rubocop cops which have no violations
Related Tickets & Documents
#2021
-
@dabrorius refactored the
show
action in ourDashboardsController
by extracting thefollowing
action out of it. Thanks, @dabrorius!
Extract "following" action from "show" in DashboardsController
#2212
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Related Tickets & Documents
Currently several separate pages are all crammed under show
action of DashboardsController.
This is the first step in an attempt of refactoring that action and separating it into several smaller controller actions.
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
[optional] What gif best describes this PR or how it makes you feel?
- @lightalloy fixed a few Rubocop issues. Thanks, Anna!
Rubocop fixes
#2232
What type of PR is this? (check all applicable)
- [x] Refactor
Description
- fixed a couple of rubocop issues
- removed the fixed and the same as default parts from the
.rubocop.yml
- moved disabled cops that need to be fixed from the
.rubocop.yml
to.rubocop_todo.yml
Related Tickets & Documents
#2021
- @jess resolved an issue by clearing old caches when merging users. Thanks, Jess!
Clear Cache When Merging Users
#2229
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
- Update counter cache
- Keep older created_at date
- Bust fastly cache
- @lightalloy started optimizing the sign in process by adding some uniqueness indices. Thanks, Anna!
Optimize sign in: Identity #2061
#2231
What type of PR is this? (check all applicable)
- [x] Refactor
Description
- added missing unique indexes on
Identity
- removed redundant unique validation (
provider
+uid
vsuid
+provider
) - made uniqueness validation conditional: only check for uniqueness when one of the fields of the scope was changed This is a slight optimization to start with for the ticket #2061
Things to consider before merging:
- Check for existing invalid
Identity
records, they need to be fixed -
identities
table could be large, so adding indexes may take a long time
-
@jess added HTML form validation to the tag weight forms to prevent
nil
values. Thanks, Jess!
Add form validation for follow 'points'
#2236
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x ] Bug Fix
- [ ] Documentation Update
Description
- If a user submits a nil/blank field for follow weight on dashboard/following, it'll give them a 500 next time they try to visit.
- This adds a form validation but no validations on the server side. We should probably do that too but when i added presence: true on the Follow model, it did some weird stuff so thought getting a quick fix in made more sense.
- @lightalloy resolved a logic error that caused some new comment notifications were not being sent. Thanks, Anna!
Fix notifications about the new child comments
#2233
What type of PR is this? (check all applicable)
- [x] Bug Fix
Description
When a child comment is posted, the article (commentable) author is notified about it only if there are no other users to notify about this comment.
I suppose that sending a notification to an article author should not depend on if it's sent to other users.
If the receive_notifications
is true, the article author should be either notified about the child comments or not notified about them.
I added a couple of tests to illustrate this issue and a fix assuming that the article author should be notified about the child comments.
Fixed sticky nav summary color to use custom property.
#2240
What type of PR is this?
- [ ] Refactor
- [ ] Feature
- [X] Bug Fix
- [ ] Documentation Update
Description
The sticky nav summary text is black, even in dark mode. This fix lets it use the custom property, with black text color as a fallback.
Screenshots
Before
After
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [X] no documentation needed
- @maestromac removed controller tests in favor of request tests. Thanks, Mac!
Remove controller specs
#2224
What type of PR is this? (check all applicable)
- [x] Refactor
Description
- Moves controller specs to request spec.
- Remove rails-controller-testing gem.
- Reorganize article request specs.
- Add additional specs.
Related Tickets & Documents
Resolves #1827
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
n/a
Added to documentation?
- [x] no documentation needed
Fixes Github Repositories not visible #2227
#2241
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
Add theme container background color to GitHub repositories container in settings/integrations page. Used background color variable used by sidebar containers.
--theme-container-background
Related Tickets & Documents
#2227
- @rhymes fixed an error with articles API where sending a request with an unknown username would return a 500 error. The PR updated it to return an empty array instead. Thanks, @rhymes!
Fix error with articles API and unknown username
#2247
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
Currently the articles API breaks with a 500 error if the server is called with an unknown username because not_found
is a controller method, not part of the service.
I believe it should return nothing like when the API is called with an unknown tag.
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- @rhymes refactored the article API endpoints a bit to only show published articles. Thanks again, @rhymes!
Add published in the SQL query to retrieve article
#2248
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Since we're already asking the DB to retrieve an article, we can directly ask it to filter out published articles without checking at runtime.
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
-
@dabrorius refactored the
show
action in ourDashboardsController
by extracting thefollowers
action out of it. Thanks, @dabrorius!
Extract "followers" action from "show" in DashboardsController
#2270
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
This is the second step in refactoring the DashboardsController
in which followers
action is separated from the generic show
action.
I kept all the route names the same for now, we might want to update them in a follow-up PR.
Related Tickets & Documents
This is a follow-up PR to this one: https://github.com/thepracticaldev/dev.to/pull/2212
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
[optional] What gif best describes this PR or how it makes you feel?
- @mjraadi fixed a miscolorization of a post's preview snippet at the top of the post's comment section. Thanks, @mjraadi!
Fixes night mode issue #2244
#2285
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
- [x] Patch Night Mode Styling
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Here are the styling fixes:
Closes: #2244
- @lightalloy added a uniqueness index to user's emails. Thanks, Anna!
Optimize sign in: User
#2237
What type of PR is this? (check all applicable)
- [x] Refactor
Description
- added a unique index to
users
email - validate user email and username only if the corresponding field changed
Need to check that there are no users with duplicate emails or usernames.
I would also like to change twitter_username
and github_username
validations to be conditional, but that would require to change allow_blank
to allow_nil
invalidations, update blanks to nils and make sure there won't be any attempts to write blank values to the database.
Related Tickets & Documents
#2061
- @ben made an optimization by validating a user's RSS feed URL only if it changed. Thanks, Ben!
Conditionally run feed validation
#2292
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Checking RSS feed validation is pretty costly in the current setup. We should only do it if something has changed with it.
- @lightalloy added uniqueness indices to Twitter and GitHub usernames. Thanks, Anna!
Unique twitter and github usernames #2061
#2255
What type of PR is this? (check all applicable)
- [x] Refactor
Description
I made this pr to prepare to change unique validations for the User
github
/twitter_username
s to be conditional. That will help with #2061
- set github and twitter usernames to
nil
in case they are an empty string: in theUser
model and in theAdmin::UsersController
(just in case) - added unique indexes to these fields in the database
Before merging:
- make sure there are no records with duplicate
twitter_username
s orgithub_username
s in theusers
table - make sure there are no records with empty strings values in these fields (update them to nil in this case).
Related Tickets & Documents
#2061
Add tests to PodcastEpisodesController
#2297
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
This PR does two things:
- adds tests for the Api::V0::PodcastEpisodesController
- removes unused routes and code
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
Fixes Upload Video Section Background Color in Night Mode Theme #2245
#2296
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
- [x] Patch Night Mode Styling
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Closes: #2245
- @lightalloy moved new comment notifications to a separate job. Thanks, Anna!
New comment notifications to Active Jobs #1996
#2287
What type of PR is this? (check all applicable)
- [x] Refactor
- [x] Bug Fix
Description
- moved sending notification about a new comment to a separate ActiveJob
- ActiveJob receives
comment_id
instead of aComment
instance. That fixes the possible issues with trying to send notifications about a destroyed comment. - moved sending notification about a new comment logic to a service object
- specs for the comment notifications functionality
Related Tickets & Documents
#1996 #1621
Fix Blogcast styling issues
#2298
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
I noticed the Blogcast embed wasn't displaying nicely on mobile devices, and it was also creating some weird spacing around it. This PR should fix both of those problems.
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- @maestromac migrated all our feature tests to system tests. Thanks, Mac!
Migrate feature test to system test
#2301
What type of PR is this? (check all applicable)
- [x] misc
Description
- Switch from feature spec to system spec
- which allows us to removed Database-cleaner and also needs to downgrade Capybara.
- Fix a few rspec inconsistency.
Related Tickets & Documents
n/a
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
n/a
Added to documentation?
- [x] no documentation needed
-
@rhymes refactored the
AnalyticsService
to use SQL queries instead of in-memory calls. This is an ongoing refactor and there's a decent discussion going on about implementation details. Feel free to chime in if you're interested! Thanks again for the help, @rhymes!
Simplify AnalyticsService code
#2308
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
This refactoring does mainly three things:
- simplifies
AnalyticsService
interface - uses SQL queries instead of in memory calculations to retrieve counts
- uses date objects for ranges
A few things I've left out that should be discussed/decided:
-
AnalyticsService
should probably be renamed toAnalytics::Pro
as suggested by @lightalloy -
date.strftime("%a, %m/%d")
is locale dependent and potentially repeats itself (if start_date and end_date go past a year there could be dates with the same string version). Also, users outside the US could misinterpret dates as "03/04" (3rd of march or 4th of april?). I suggest using the ISO "2019-04-01" as key for the hash. - I think the date validation using regexps, now that dates are parsed, is unnecessary:
def valid_date_params?
date_regex = /\A\d{4}-\d{1,2}-\d{1,2}\Z/ # for example, 2019-03-22 or 2019-2-1
if params[:end]
(params[:start] =~ date_regex)&.zero? && (params[:end] =~ date_regex)&.zero?
else
(params[:start] =~ date_regex)&.zero?
end
end
Related Tickets & Documents
Closes #2307
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- @lightalloy added conditional validations for a user's GitHub username and Twitter username. Thanks, Anna!
Conditional validations for user github_username and twitter_username #2061
#2312
What type of PR is this? (check all applicable)
- [x] Optimization
Description
- validate
github_username
andtwitter_username
uniqueness only if the corresponding field was changed to lower amount of requests when signing in and in other cases - change
allow_blank
toallow_nil
options in these fields' validations, since onlynil
is allowed to be written to the database.
Related Tickets & Documents
#2061
New Issues and Discussions
-
@glebec raised an accessibility concern where a
diff
code block syntax highlighting could use more accessible colors. Thanks, @glebec!
Markdown > code block > `diff`-format syntax highlighting: use accessible colors
#2087
Is your feature request related to a problem? Please describe.
The diff
syntax for language blocks in markdown uses red and green as the contrasting colors for deleted and introduced lines respectively. Over 5% of men and ~0.5% of women have difficulty distinguishing red and green, meaning that some people will struggle to tell apart such lines, relying more on the small -
and +
characters on each line.
Example in GitHub (which also uses red-green for many UI elements):
this is an unchanged line
- this is a removed line
+ this is a new line
this is more unchanged stuff
Describe the solution you'd like An alternative would be to modify the CSS for highlighting diffs to use variations on orange and blue respectively, which a much higher percentage of the population can distinguish.
Describe alternatives you've considered
- Red-blue is an imperfect compromise, better than red-green but not as universally distinguishable as orange-blue. However, red-blue might fit in better with cultural expectations of meaning, producing less confusion overall.
- Alternatively, no action may be taken. This isn't the most accessible choice, but as long as diffs still include ± characters in them, they are technically readable.
Additional context Mockup below; exact values are not literal proposals, just food for thought.
Before:
After:
- @co16353sidak reported a bug in night mode when hovering on Reading List Section
Bug(Night Mode):- Hover Issues on Reading List Section
#2121
Describe the bug
In the night mode, there seems to be some problems with the hover on the reading list section of the page.
Shown below. when out cursor moves into the Reading List section, the background colour of the lower section containing the buttons saved posts
and comment activity
unexpectedly changes colour to merge with that of the background.
If we continue over to actually hovering over the buttons, then the whole section completely changes it's background colour to merge with the page.
I have included a gif to help understand this.
Also if we compare with the functioning of the default mode, we can see the difference and how this is supposed to work.
To Reproduce Steps to reproduce the behavior:
- Enable Dark Mode
- Goto the main
dev.to
homepage - try hovering over the elements mentioned (the
reading-list
section) to see the described error
Expected behavior I believe it should work similarly like the default mode, but with different colour variables depending on the mode selected, I have included a gif for the same on default mode.
Screenshots Linked above.
Desktop (please complete the following information):
- OS: MacOS
- Browser: Safari and Firefox
- Version: 12.0.3 (14606.4.5)[latest] and 67.0b3 (64-bit)[latest developer edition]
Smartphone (please complete the following information):
(Not Tested)
-
@borisschapira reported an issue where
src=/"data
was causing a Markdown error. Thansk, @borisschapira!
Markdown code contribution error
#2126
Describe the bug Unable to write a post containing HTML code for an image.
To Reproduce Contribute this in an new post:
---
title: 'Test post'
---
```
<img src="data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==" …
```
Expected behavior Saving is possible.
Desktop (please complete the following information):
- OS: mac os
- Browser Chrome
- Version 73.0.3683.75
- @peter requested a feature where you can sort the dashboard by views/reactions. Thanks, Peter!
Add ability to sort articles on dashboard by views/reactions
#2134
Is your feature request related to a problem? Please describe. As an author, I'd like to be able to sort my posts by criteria such as views / engagement / comments count, instead of strictly viewing in chronological order.
Describe the solution you'd like An option on the dashboard which allows me to filter by certain criteria (views, reactions, comments).
- @joppedc reported an issue where their profile picture was narrowing in the preview mode of a post. Thanks, @joppedc!
Squished profile picture on preview
#2137
Describe the bug When doing a post preview, the profile pic next to the publisher name is squished.
To Reproduce Create a new post, preview it.
- Go to write a post
- Click on preview
- See publisher image
Expected behavior Normal image
Screenshots https://cl.ly/8950bcf37c07
Desktop (please complete the following information):
- OS: MacOS
- Browser: Chrome
- Version 72
Additional context This issue might be good as a first issue or a starter issue.
- @kontrollanten requested a feature to display the bookmark inside "Save" button for each post in the feed. Thanks, @kontrollanten!
Display bookmark icon inside "Save" button
#2144
Is your feature request related to a problem? Please describe. I've used dev.to for a while now, and I like a lot. It took my a while though to understand what the bookmark icon is. For a long time I thought it was just a "reaction". So every time I found a post I'd like to save, I had to search for the hashtag and in the list click on the "Save" button, to save it for later. And many other posts I "spam" reacted to with heart, unicorn and ...bookmark. So I was a bit confused for a while.
Describe the solution you'd like I'd like to make it clearer that clicking on the bookmark icon in a article detail has the same effect as clicking on "Save" button in the list view.
Describe alternatives you've considered I'd like to add the bookmark button inside the Save button. I'm not a designer or UX, so I don't know if this has other effects thought.
width: auto;
min-width: auto;
margin-right: 5px;
height: 19px;
margin-left: 0;
vertical-align: -4px;
- @rhymes raised an issue where our static assets have unusually short caching period. Thanks, @rhymes!
Static assets have unusually short caching period
#2147
Describe the bug
I was testing the homepage with Google PageSpeed Insights and WebPage Performance Test (both links lead to the reports) and they both signalled that static assets (JavaScript files, images and CSS) have short caches, ranging from 1 day to 7 days.
Since they are all versioned I was wondering if there was a reason for that otherwise I'd like to suggest to set a longer cache for those, following Google's best practice and set the cache to one year.
Expected behavior
A longer cache period for static assets.
Screenshots
Google PageSpeed Insights (same as Lighthouse):
WebPage Performance Test:
Desktop (please complete the following information):
- OS: macOS
- Browser: Firefox
- Version: 67b3
- @link2twenty raised an issue about the contrast of special text in night mode. Thanks, @link2twenty!
Contrast on special text in night mode
#2152
The new spring banner is difficult to read in night mode. Is there a way to automate contrast checks? Or should we just be mindful when making special styles?
- @link2twenty opened the discussion about having Ruby files specifically for themes. Thanks, @link2twenty!
Ruby themes file
#2154
Is it worth having a ruby file that contains the theme's HTML, though we'd only have night mode in there for now?
I'm not that clued up on ruby on rails but I presume we can have a file that contains several thems but will only return the relevant HTML based on some request?
CC: @rhymes @lightalloy
- @rhymes raised an security issue regarding missing HTTP strict transport security headers (HSTS). Thanks, @rhymes!
Add HTTP strict transport security headers (HSTS)
#2155
Is your feature request related to a problem? Please describe.
By playing with dev.to's report of Mozilla Observatory I noticed the website does not send HSTS headers.
The TLDR; of that is that the HSTS header forces the client to connect only using HTTPS (which is different from the server side redirect from HTTP to HTTPS).
Describe the solution you'd like
I'll summarise the recommended plan of action:
- all domains and subdomains of dev.to should be checked to make sure they all work in HTTPS (even those handled by third party like shop.dev.to)
- ramp up the max-age in steps, for example from
max-age=300; includeSubDomains
to a month:max-age=2592000; includeSubDomains
- once everything works you can set it the recommended two years
max-age=63072000; includeSubDomains; preload
and add the domain to the HSTS preload list
Header example:
Strict-Transport-Security: max-age=xyz
With Rails it would be enabled like this:
config.ssl_options = { hsts: { expires: 5.minutes } }
Options to enable subdomains and preload are also present.
Resources:
- Mozilla Observatory results for dev.to
- HTTP Strict Transport Security - Mozilla Security guide
- How to deploy HSTS safely in stages
- HSTS preload and subdomains
- HSTS in Rails
Describe alternatives you've considered
Doing nothing :D
- @link2twenty raised an issue about the color of the notification bell in the top navigation bar in night mode. Thanks, @link2twenty!
[Theme] Notification color
#2156
Do we want to theme the bell number?
If we do should it be the same as --theme-top-bar-write-background
or perhaps a new property, --theme-top-bar-notification-background
?
- @link2twenty also opened a discussion on how a user could select themes. Check out the cool mockup! Thanks again, @link2twenty!
[Discussion] Theme selector
#2158
At some point we'll have to talk about theme selection, I find a graphic is better than a name as it gives the user an idea of what to expect.
I made a quick mock-up. https://jsfiddle.net/link2twenty/3pmtew1f/
- @aturingmachine raised an issue with night mode where external links in posts are difficult to see. Thanks, @turingmachine!
External Links In Posts Are Difficult To See In Dark Mode
#2163
Describe the bug Links are only marginally different in color than the post's text. Coupled with the dark background changing the overall contrast of the page, this makes them difficult to see.
To Reproduce Steps to reproduce the behavior:
- Log in with Dark Mode Enabled
- Click on any post including external links.
- Observe link text color.
Expected behavior The link text to be more easily differentiated than the post's main body text.
- OS: MacOS
- Browser Chrome
- Version N/A
Use only one version of CodeMirror
#2165
Is your feature request related to a problem? Please describe.
The codebase uses various versions of codemirror:
-
package.json
declares^5.44.0
andyarn.lock
is using5.44.0
. This is the version used by the code editor:
-
app/views/blocks/_form.html.erb
links to another version of CodeMirror,5.26.0
:
-
app/views/chat_channels/index.html.erb
uses a third version of CodeMirror (only the CSS),5.38.0
:
Describe the solution you'd like
CodeMirror is currently at version 5.45.0. I was wondering if all three sections could use the same version from the package.json
- @john_papa raised an issue with night mode for the admin section for organizations. Thanks, @john_papa!
- @equiman reported an issue with night mode for the Social icons in profile pages. Thanks again, @equiman!
- @equiman reported an issue with night mode for the "Following tags" section in your dashboard. Thanks again, @equiman!
- @rhymes opened a discussion about whether we should use randomly generated IDs in the future REST API. Thanks, @rhymes!
Use randomly generated IDs in the REST API
#2187
Is your feature request related to a problem? Please describe.
I'm opening this ticket after having read this great article about data analysis on dev.to's articles.
The question I'd like to set forth, in preparation for a future "publication" of the API, is: should dev.to use randomly generated resource IDs instead of exposing "internal" autoincrementing integer primary keys or not?
A "classic" best practice in API design is not to expose internal details of your app, separating as much as possible how the data is organized from how is accessible from third parties.
Autoincrementing primary keys are usually discouraged in public APIs (by some, not by all) mainly for three reasons:
- they expose database IDs to the public. Today you might have data in a relational database, tomorrow that data might be elsewhere and those IDs might not mean much because autoincrementing primary keys are local to one data source.
- by exposing incrementing IDs an onlooker (or a malicious attacker) can gather two information: size of the tables and growth rate
- it's easier for a malicious attacker to write a script to scrape data
This is more a question of policy than a technical argument because, at least for the second of the three reasons, DEV is perhaps perfectly fine with that.
Some possible alternatives to auto incrementing integer primary keys:
- UUIDs: PostgreSQL has a native indexable UUID type with functions to generate default UUIDs, like pgcrypto's
gen_random_uuid()
. UUIDs though are not sortable, they kind of look ugly and they are verbose:https://dev.to/api/articles/2d931510-d99f-494a-8c67-87feb05e1594
- KSUIDs: they are quite new and were "invented" by Segment. The characteristic they have is that they are sortable and time dependent:
https://dev.to/api/articles/0ujsszwN8NRY24YaXiTIE2VWDTS
- some other random alphanumeric ID generated in the app (eg. Ruby's
SecureRandom.alphanumeric(20)
):https://dev.to/api/articles/cDAkKyz38cqjTI1bV5lG
My preference goes to one of the last two options
Describe the solution you'd like
Because the API is not public yet, and it's relatively small (only 8 controllers), I think that the transition would be doable if there's consensus on this. After publication and documentation it might be harder, especially because DEV is already a big community and developers are already jumping at the opportunity to use the API (hence the article ;))
For obvious reasons there shouldn't be a situation where both (integer IDs and alphanumeric IDs) would work at the same time. That would only strain the system (because it might result in two queries, instead of one).
The frontend/SPA would also need to be aligned and updated.
What do you think?
Additional context
I did a quick tour of some public APIs (starting from those used by DEV itself and this is what I've found:
-
Fastly uses both URLs (for obvious reasons) and alphanumeric IDs (eg.
SU1Z0isxPaozGVKXdv0eY
) to identify resources, probably generated by the equivalent of Ruby'sSecureRandom.alphanumeric(21)
-
Cloudinary uses a
public_id
which is a randomly generated string (eg.8jsb1xofxdqamu2rzwt9q
) -
Stripe uses alphanumeric random strings prefixed by a resource identifier, for example
ch_19yUdh2eZvKYlo2CkFVBOZG7
for a charge andcus_El7v7DRE34iBPx
for a customer
A few more:
- StackOverflow uses integer IDs
- Twilio uses string IDs with a prefix for type of resource, similar to Stripe
- PayPal uses alphanumeric IDs
Refs #911
- @kenbellows reported an issue where certain characters, like angle brackets, show up incorrectly in some article previews. Thanks, @kenbellows!
Certain characters, like angle brackets, show up incorrectly in some article previews
#2204
Describe the bug
When viewing the /comments
view of an article, the first line and a half or so of the article is displayed. If that first bit contains inline code with angle brackets, e.g. if you wrote:
this is an article about `<span>` tags
this appears in the article preview as: "this is an article about <span> tags".
To Reproduce Steps to reproduce the behavior:
- Go to 'https://dev.to/new'.
- Enter minimal front matter, set `published: true, and enter:
this is an article about `<span>` tags
- Save the article.
- Navigate to {article URL}/comments.
- Observe the
<span>
in the preview text.
Expected behavior
I expect the preview to say "This is an article about <span>
tags"
Screenshots
Example from: https://dev.to/kenbellows/stop-using-so-many-divs-an-intro-to-semantic-html-3i9i/comments
Desktop (please complete the following information):
- OS: macOS Mojave 10.14.3
- Browser: Chrome 72.0.3626.121, Opera 58.0.3135.107
- @john_papa reported an issue with night mode about how code in comments need contrast. Thanks, @john_papa!
- @learnbyexample reported an issue where code block with string containing ⭕ gets changed to the emoji. Thanks, @learnbyexample!
Code block with string containing ⭕ gets changed to ⭕️
#2216
Describe the bug
Code block with string containing :o:
(lowercase letter that comes after n
surrounded by colons) gets changed to ⭕️
(perhaps an emoji?)
To Reproduce
Using any of these code blocks in a post/comment and previewing it shows the issue. Having any other characters around this sequence doesn't matter as far as I've checked.
```
:o:
```
```python
':o:'
```
```ruby
':o:'
":o:"
```
Expected behavior
Within code blocks, I do not expect any string combination (that I know of) to be interpreted and converted to another string. :o:
should just be :o:
not ⭕️
Screenshots
The above code blocks produces this output:
- @healeycodes reported an issue with night mode where embedded comments are hard to read. Thanks @healeycodes!
Bug(Night Mode): Embedded comments are hard to read
#2221
Describe the bug With Night Mode enabled, embedded comments in articles are hard to read.
They show up as black text against a dark blue background.
To Reproduce With Night Mode enabled, go to an article that embeds comments.
Such as https://dev.to/devteam/top-5-dev-comments-from-the-past-week-gia
Screenshots
Desktop:
- OS: Windows 10 OS Build 17134.648
- Browser: Chrome
- Version: 73.0.3683.86 (Official Build) (64-bit) (cohort: Stable)
- @andypiper reported an issue with night mode where an organization's CTA box content is hard to read. Thanks, @andypiper!
Dark mode - org CTA box content is hard to read (needs contrast)
#2222
Describe the bug It is difficult to read the black text on dark background in the CTA box on organisation posts.
To Reproduce Steps to reproduce the behavior:
- Go to https://dev.to/twilio
- Click on any post
- Check the sidebar
- See below
-
@lito requested a feature where we add an HTML
<meta>
tag with the current deployed git commit hash. Thanks, @lito!
Add a HTML Meta tag with deployed git commit hash
#2223
What about to add a html
meta
tag on header with the current git commit deployed?
On this way it's easy to contributors and developers follow website status and deployment compared with github commits.
I don't know if exists some tag for this proposse, but what about something like:
<meta name="git-commit" content="6e23483b9421c1e1a03d94093306badf277cc19e">
Regards :)
Non english characters are deleted on tags
#2226
Describe the bug When you write tags with some non english characters, these characters are deleted on article view.
To Reproduce
- Use a tag like
Optimización
(Optimization). - Save
- Go to article
-
Optimización
is nowoptimizacin
Expected behavior
Show Optimización
as in markdown editor.
- @brylie reported an issue where service workers and a privacy extension was blocking images from loading. Thanks, @brylie!
Error loading images
#2228
I shared a dev.to article on Reddit, and one of the commenters said images wouldn't load.
Here is the rest of their diagnosis:
These images:
https://i.imgur.com/GArpxyC.png
Errors reported in console:
manifest-36a1d0b68d1e3664106b.js:1 Uncaught SyntaxError: Unexpected token <
vendor-a16759536b738a97f545.js:1 Uncaught SyntaxError: Unexpected token <
Search-d6be81c747f6325b6aae.js:1 Uncaught SyntaxError: Unexpected token <
base-55719a78ce4c394706e03834ae49c59cddc235c3ef5dbc299c63b4edf0f0b558.js:1 Uncaught SyntaxError: Unexpected token <
webShare-1698d2d09639b4c8b299.js:1 Uncaught SyntaxError: Unexpected token <
reload-router-view-when-vue-route-changes-3cck:1 Error while trying to use the following icon from the Manifest: https://practicaldev-herokuapp-com.freetls.fastly.net/assets/devlogo-pwa-192-b92c1c02f2f63967c2889ba04a4d7bc61da640a6dbe419d65703b1282c96b46c.png (Download error or resource isn't a valid image)
serviceworker.js:1 Uncaught (in promise) TypeError: Failed to fetch
If I go to "Google chrome dev tools" --> "Application" --> "Service worker" and check "Bypass for network", and reload, everything works
After some more debugging, it seems like the XHR requests to *.freessl.fastly are blocked by the privacy settings in my browser, if I whitelist that origin for XHR requests from your domain, it works
- @lightalloy raised the issue that merging user accounts should be done in a database transaction. Thanks, Anna!
Merging user accounts should be done in a transaction
#2230
I think that for a safer merge at least some of the actions should be done inside a db transaction.
- @ben requested a feature where we v2 editor could also handle plain front matter. Thanks, Ben!
Toggle plain frontmatter editor within v2
#2234
Is your feature request related to a problem? Please describe. Currently if you want to write "simple" markdown, you do so with the original editor, which is fine, but has always been a bit weird and occasionally buggy.
The new editor has rich UI controls, but some folks still want the markdown pasting experience (for a lot of valid reasons beyond just taste!)
It would be nice if "v2" (the preact-built editor) supported our whole range of use cases and we could sunset the old code (perhaps to be maintained by the community)
Describe the solution you'd like
Any area to toggle between editor types would be great. Users could have a default editor type but easily switch when needed.
- @antonrich requested a feature where you can combine tags together in a search query.
Combining tags together in a search query.
#2238
It would be great if users could write a search query of the following format: #tag1 #tag2 and receive the results that actually have such tags.
https://dev.to/antonrich/how-can-i-combine-tags-when-searching-on-dev-to-5b19
- @mte90 Improve the feed using the tags picked by an user
Improve the feed using the tags picked by an user
#2249
Describe the bug An user pick the tags that want to follow but there isn't any priority to them on the various feed availables.
To Reproduce Steps to reproduce the behavior:
- Set tags on your user
- Check the tags of the post in your feed
Expected behavior The feeds has only stuff of the tags that the user is following or else they are useless right now
-
@rhymes reported a strange issue with the API where
https://dev.to/api/comments?a_id=some_id
wasn't working, even though there seemingly isn't anything wrong with the related code. Thanks for reporting this, @rhymes!
API: can't get comments belonging to an article
#2250
Describe the bug
I can't seem to be able to retrieve articles comments from the DEV API. It works on my local installation but not using the live API.
Given the article API response https://dev.to/api/articles/95907 - https://dev.to/kathyra_/what-security-through-obscurity-is-and-why-it-s-evil-47d5 - I tried to retrieve its comments using https://dev.to/api/comments?a_id=95907 but the server returns HTTP 404
.
At a first glance it should work, according to the code:
The odd thing is that such call works correctly on a local installation.
I'm probably doing something wrong on my end.
To Reproduce Steps to reproduce the behavior:
- Go to https://dev.to/api/comments?a_id=95907
- See error
Expected behavior
I'd expect this endpoint to return all the comments of the article with id 95907
which should be the following: https://dev.to/kathyra_/what-security-through-obscurity-is-and-why-it-s-evil-47d5
Screenshots
- @mjraadi reported an issue with night mode where the comment preview text is unreadable. Thanks, @mjraadi!
[Night Mode]: Article's Comment Preview Needs to be Fixed
#2286
Describe the bug Comment preview of an article needs to be updated according to the night mode color scheme. Background color is yellow and font color is white and the text is unreadable.
To Reproduce Steps to reproduce the behavior:
- Go to any article
- In the comment box type anything
- Click on "Preview"
- See the behavior
Expected behavior Contrast between background color and text color.
Desktop (please complete the following information):
- OS: Arch Linux Manjaro
- Browser Chrome
- @learnbyexample reported an issue where inline code with double backslash becomes a single backslash if it is part of nested bullet list.
Inline code with double backslash becomes single if it is part of nested bullet list
#2288
Describe the bug
Double backslash inside code blocks in nested listing results in single backslash instead of two.
To Reproduce
From any post/comment, try these non-nested and nested bullet lists:
* foo
* use `\\1`, `\\2`, etc
---
* foo
* baz
* use `\\1`, `\\2`, etc
Expected behavior
Double backslash shouldn't become single, as shown below for GitHub markdown rendering:
- foo
- baz
- use
\\1
,\\2
, etc
Screenshots
Screenshot for above non-nested (correctly rendering) and nested list (incorrect):
Not sure if this is related to: https://github.com/thepracticaldev/dev.to/issues/960
- @jess reported an issue where a Code Sandbox Liquid tag w/ focus input causes the page to jump. Thanks, Jess!
Code sandbox liquid tag w/ focus input causes scrolling on page load
#2290
Describe the bug Reported by a user trying to embed a codesandbox that focuses an input. The page 'jumps'/scrolls to the embed when it loads.
To Reproduce
Create a new post w/ this liquid tag: {% codesandbox pyqq0o9mk7 %}
. Make sure it appears below the fold so you can see the jump.
Expected behavior Liquid Tag shouldn't cause scrolling on page load.
Dark Mode text weight/kerning
#2295
Is your feature request related to a problem? Please describe. I'm in no way visually disabled, but reading large groupings of text in nightmode makes my eyes hurt!
Describe the solution you'd like A lighter font weight for regular text in dark mode. Details here:
Describe alternatives you've considered Unsure, I imagine this will be far far far down on the totem pole anyhow.
Additional context
This showcases how the font weight is hard to distinguish and how "clumped" the text is.
Consider making reading times an 'opt-in' feature
#2304
Is your feature request related to a problem? Please describe.
People with dyslexia may find reading times unpleasant:
Describe the solution you'd like Suggest that the reading time either be removed or provide an option to opt-in to see them (so that by default they are hidden).
Describe alternatives you've considered See above - either remove reading times or provide an opt-in to enable them (cookie?)
Additional context
Here is one blog post showing the reading time:
- @rhymes opened a discussion about possible improvements and ideas for handling the data of our analytics. Thanks, @rhymes!
Analytics service: possible improvements and ideas for the future
#2311
Is your feature request related to a problem? Please describe.
This has been opened to collect all possible improvements, in case of need, for the analytics API after questions about performance have been raised in #2307 prior to the refactoring in #2308 (AFAIK yet to be measured/verified for the use case that lead to #2307).
Describe the solution you'd like
There are possible incremental solutions (not all alternatives to each other, some can be applied incrementally). Some suggested solutions require a trivial amount of effort, others require more work.
Theoretically all should be measured against the status quo to understand if needed or not.
Possible solutions
Hard limiting the date range of analytics that can be requested
The proposed solution would forcibly limit how wide the range of dates for the analytics can a client request. As mentioned by @Zhao-Andy some public API's like Twitter limit requests up to 30 days.
Pros
- setting a cap on the amount of data a client can request is a good idea for obvious reasons
- most analytics clients will use rolling windows on dates anyway or are interested in a specific period
Cons
- would probably require special care if any client is to be treated differently (for commercial partnership where the client asks to access more data at same time)
- it could lead to more HTTP requests for clients interested in a wider range and hence more queries if the client is after data for more than 30 days
My recommendation
I think it's a great idea and should definitely be considered, though applied as a result of a clear data policy (maybe even explicitly explained in the documentation) not just for performance reasons (those if encountered should be solved at the best of our abilities, not just "hidden" by limiting the date range)
Adding indexes to columns in where conditions
Most queries in the AnalyticsService
work on created_at
columns of the tables reactions
, comments
, page_views
. The proposed solution is to add indexes on those columns to improve speed of retrieval for large data sets.
Pros
- a request that asks for a large number of articles over a large interval of date should be aided by indexes
Cons
- indexes can slow down writes for tables with a high frequency of writes
- as it currently stands the code splits the queries over each day, the difference in most cases might not be noticeable
My recommendation
I think this is a quick win and should seriously be considered as a companion to any other solution. Where conditions on dates are a classic place where an index can benefit. Ideally one show open the SQL console and measure the before and after of the query with explain analyze but it's not mandatory. I also don't believe, AFAIK, there's going to be any impact on writes because what I perceive to be the biggest table among the three, reactions
, doesn't get millions of INSERT
s at a time. Anyhow that can be monitored with the slow query tab on Heroku in case the problem presents itself in the future.
Use grouping
The proposed solution is to let the DB group data and return aggregates instead of computing aggregates with following queries per each day in the range of dates.
Pros
- the database is generally really good at mixing grouping and where conditions (provided there are indexes on the needed columns)
- it would lower the amount of queries
Cons
- it requires testing. Ideally one should write a test that populates the DB with a data sample over 2 or 3 days, write an assertion/expectation about the result, then go about improving the internals of the service to make sure the expected data matches the new structure of querying (usual testing, nothing to see here hehe)
- it requires some trial and error and knowledge of PostgreSQL group by clause (the link points to a tutorial)
My recommendation
I'm obviously biased in favor of this (let the DB do its job as a mantra) if the expected result allow for it (I haven't looked deeply into it so it might not be feasible). Rails natively supports GROUP BY
and HAVING
(both clauses needed to grouping and filtering on groups) through ActiveRecord methods group and having. @lightalloy has mentioned the existence of the gem groupdate geared specifically at grouping by dates using an agnostic interface that works on all supported DBs, that could come in handy for this and future date based aggregations (though the syntax in vanilla Rails is pretty straightforward:
[7] pry(main)> PageView.where(created_at: 2.days.ago..Time.current).group("date(created_at)").sum(:counts_for_number_of_views)
(0.8ms) SELECT SUM("page_views"."counts_for_number_of_views") AS sum_counts_for_number_of_views, date(created_at) AS date_created_at FROM "page_views" WHERE "page_views"."created_at" BETWEEN $1 AND $2 GROUP BY date(created_at) [["created_at", "2019-04-03 08:47:39.869415"], ["created_at", "2019-04-05 08:47:39.869509"]] [sql_query]
=> {Thu, 04 Apr 2019=>7}
you can verify how the query works correctly by a quick look at the data:
[8] pry(main)> PageView.all.pluck(:counts_for_number_of_views, :created_at)
(0.5ms) SELECT "page_views"."counts_for_number_of_views", "page_views"."created_at" FROM "page_views" [sql_query]
=> [[1, Thu, 04 Apr 2019 21:28:07 UTC +00:00],
[1, Thu, 04 Apr 2019 21:28:56 UTC +00:00],
[1, Thu, 04 Apr 2019 21:39:25 UTC +00:00],
[1, Thu, 04 Apr 2019 22:39:33 UTC +00:00],
[1, Thu, 04 Apr 2019 22:41:03 UTC +00:00],
[1, Thu, 04 Apr 2019 22:43:49 UTC +00:00],
[1, Thu, 04 Apr 2019 22:45:07 UTC +00:00]]
Pre-compute data using batching and compute the difference in real time
The proposed solution is to add an analytics table that contain aggregated data populated by a scheduled recurring job
Pros
- this has the obvious pro of having near instantaneous read time. The client asks for a date range, a
SELECT
is issued to the aggregated table and the data is returned - it can lead to further optimizations down the line (if combined to caching or other changing business requirements I can't foresee right now)
Cons
- it requires a bit of effort upfront
- it requires a merge function (if the aggregated data is less than the required data) that still has to go to the live tables
- it may require an eviction policy (how long are aggregates kept in storage?)
- what happens to the old rows of aggregated data if there's a bug in the analytics function or if new metrics are added?
My recommendation
I'd consider this when the other options have been exhausted. If there's a massive amount of data continuously requested by HTTP clients and indexes and grouping are failing you, then a switch in the approach is a way to go. I'm always a big reluctant into going "batching first" because most of the times there are avenues to be exhausted before. Batching also requires some logic on merging, eviction and coping with stale data (or having to recompute older rows in the background)
Low level caching on results
The proposed solution is to guard one or each metric with a cache, so that an individual client requiring a specific range twice will have its data returned from memory the second time
Pros
- as with batching the data can be returned quickly once the cache is primed
- by knowing in advance one could pre-compute cached values for specific users/organizations that have particularly massive computations ahead of them
- it shouldn't have a big impact on the memory occupied by the caching server
Cons
- it's unlikely that an organization is going to ask for the same date range multiple times often, hence caching might defeat the purpose (organization usually write scheduled scripts that retrieve analytics and update internal dashboards :D)
- caching individual dates would require too many trips to the caching server, and a SQL query might be faster in that case
- as with batching (which is basically a cache written to disk), caches need to be handled in the sense that if tomorrow a new metric is added, there has to be a mechanism to re-populate older caches or at least empty all existing ones, this requires a bit of analysis
My recommendation
I'd go down this route after exhausting all "live optimizations" though it can be extremely beneficial for bigger organizations or extremely popular users or organization/users whose analytics are asked in a continuos manner though usually aggregated analytics are asked for once a day by each client so it's easy to plan the impact on the systems.
DEV-iOS
We haven't had any new issues or PRs merged lately. Feel free to check out the iOS repo, or download our iOS app on the App Store.
That's it for this week! Next week, I promise we'll be covering April 6 to April 12 and be on a regular weekly schedule. 🙇♂️
Top comments (6)
Thank you for your contributions @ben , @jess , @picocreator , @link2twenty , @willamesoares , @davefollett , @rhymes , @dabrorius , @andy , @glennmen , @mariocsee , @perigk , @bolariinwa , @lightalloy , @keshavbiswa , @selbekk , @edisonywh , @venarius , @abraham , @cyrillefr, @mscccc , @brylie , @aspittel , @lito , @peiche , @maestromac , @perlatsp , @mjraadi , @m1guelpf , @glebec , @co16353sidak , @borisschapira , @peter , @joppedc , @kontrollanten , @aturingmachine , @john_papa , @equiman , @kenbellows , @learnbyexample , @healeycodes , @andypiper , @antonrich , @mte90 , @forsh3y , @jaydm !
Thanks for the ever precise summary!
It's great to see the list of contributors getting ever longer too
Glad to be part of this @andy :)
Wow, lot of awesome new features. It's been a while since I've been on dev.to much, and I'm (happily) surprised how productive the team is. Nice work!
Thanks! This is a three week issue so there's a lot more than usual. We get a decent amount of features from contributors as well!