Welcome back to another Repo Recap, where we cover last week's contributions to dev.to's repository the iOS repo, and the Android repo. This edition is covering June 8 to June 14.
Main Repo
Features
Dashboard Pro: support InstantClick and show org analytics #3102
What type of PR is this? (check all applicable)
- [ ] Refactor
- [x] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
This PR makes the Pro Dashboard work with InstantClick and displays the correct analytics for organizations.
It moves all the code inside functions, removing globals and then loads the "initializer" inside InstantClick change
event so that the transition between clicks is smooth.
Right now, if you have your own dashboard and your organization's dashboard, clicking between the two doesn't work. Also, if you go from https://dev.to/dashboard to https://dev.to/dashboard/pro from the internal link you have to refresh the page.
Another thing it does is to display the correct analytics for the organization. Right now we never pass the organization ID to the analytics business logic, which means that the data returned it's always that of the user, not that of the organization the user is looking at.
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- We've removed for the time being the number of unread organization notifications since it was causing performance. PR by @ben
Remove org notifications count from notifications view #3158
What type of PR is this? (check all applicable)
- [ ] Refactor
- [x] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
We're having some performance issues with org notifications and this is a quick fix to help it out before we get to the final fix.
Bug Fixes / Other Contributions
-
@danielcompton added a space between "Change meta tag" and
canonical_url
. Thanks, @danielcompton!
Add a space between "Change meta tag" and canonical_url #3063
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
Without the space between these two elements, this shows up as Change meta tagcanonical_url
.
Related Tickets & Documents
https://github.com/prettier/prettier/issues/4223#issuecomment-380093772 describes how composing JSX elements over multiple lines needs {' '}
.
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
I haven't actually tried this myself, but I saw the before image at https://www.cdevn.com/why-medium-actually-sucks/:
and looking at the code, I could see why it looked that way, and what to do to fix it.
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- @rhymes disabled podcast episodes from fetching during the development seeding process. Thanks for the optimization, @rhymes!
Disable Podcast episodes fetching during seeding #3075
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
ActiveJob jobs are executed inline during rails db:reset
which means that the app now that https://github.com/thepracticaldev/dev.to/pull/3057 is in, tries to fetch 1000 episodes from each Podcast, which is really slow if successful at all (it crashed after a while on my machine).
This PR disables fetching during seeding. Episodes can be fetched with rails get_podcast_episodes
task if needed for local testing.
I know @lightalloy is working on improving Podcast fetching but I believe this was an unintended side effect of the good work she's doing.
The fact that we have to disable a callback it's also a good argument against doing this kind of things in callbacks which hides a lot of business logic in the first place (by separating creation of an object and fetching of dependent objects for example) but that's an entirely different argument :-)
Related Tickets & Documents
#3057 and #2952
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- @rhymes completely refactored how we process analytics for presentation. It's a very detailed PR if you're interested in the code. Thanks again, @rhymes!
Analytics: refactoring for speed improvements #3072
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
This refactoring began when I was started taking a look at how the whole analytics worked in the Dashboard pro and noticed we could have computed it faster. I've also noticed that sometimes it requires multiple refreshes in production to be shown due to performance issues.
This PR contains two things: a refactoring of the logic in the AnalyticsController
for the API and speed improvements for the analytics themselves.
Solutions taken into account
I've talked about possible improvements in #2311 and the possible solutions envisioned were:
- limiting the date range (not implemented here)
- adding indexes to columns that are part of the filtering/where conditions (part of this PR)
- use grouping in SQL (part of this PR)
- pre computing data in batches (not implemented here)
- low level caching (this is already in production but there are issues with caching each date separately as we can already see in production. Each date means a separate network call to the caching server, which means an average of 30 HTTP calls for a month range and possibly hundreds for "infinity". I've changed it to a single cache call per range but we'll talk more about caching later)
So, this PR contains a better usage of SQL (from 64 queries to 4 in the grouping by day) and adds indexes on columns that are going to be filtered on repeatedly (indexes are added concurrently by PostgreSQL with the Rails migration not to disrupt normal production flow)
Walkthrough of the steps taken and ameliorations
First thing first I had to set a baseline. You can't make things "faster" without knowing how slow or fast are they. So I wrote a script that uses Ruby's benchmark module to have a ballpark of what was going on:
require 'benchmark'
# disable logging to avoid measuring that
ActiveRecord::Base.logger = nil
user = User.find(11)
iterations = 1_000
Benchmark.bm do |bm|
bm.report("totals") do
as = AnalyticsService.new(user)
iterations.times { as.totals }
end
bm.report("last week") do
as = AnalyticsService.new(user, start_date: 1.week.ago.to_date.iso8601)
iterations.times { as.grouped_by_day }
end
bm.report("last month") do
as = AnalyticsService.new(user, start_date: 1.month.ago.to_date.iso8601)
iterations.times { as.grouped_by_day }
end
bm.report("since the beginning") do
as = AnalyticsService.new(user, start_date: "2019-04-01")
iterations.times { as.grouped_by_day }
end
bm.report("date too much in the past") do
as = AnalyticsService.new(user, start_date: "2018-01-01")
iterations.times { as.grouped_by_day }
end
end
This was the result on my meager development instance (not too much data in the DB):
user system total real
totals 2.028109 0.231059 2.259168 ( 3.594266)
last week 27.779555 1.734052 29.513607 ( 45.470222)
last month 120.701257 7.255624 127.956881 (206.690019)
since the beginning 225.478188 13.058602 238.536790 (384.704629)
since the beginning refers to 2019-04-01
which is the date DEV released Analytics.
Step 1 - refactor totals
After refactoring the AnalyticsService.totals
calculations I benchmarked this timing:
user system total real
totals 1.787045 0.128882 1.915927 ( 2.999562)
a 1.17x speed increase over master branch. Not much but it's a start and there were less queries all around.
Step 2 - refactor grouping by day
This took considerably more time to achieve, but after playing around a bit with PostgreSQL, its query plans the number of queries went down from 64 to 4 (one per each metric).
These are the times measured:
user system total real
last week 3.164215 0.148648 3.312863 ( 4.885400)
last month 5.232762 0.173115 5.405877 ( 7.152623)
since the beginning 7.509549 0.204578 7.714127 ( 9.507002)
date too much in the past 39.811000 0.820973 40.631973 ( 44.212027)
last week: 8.91x speed increase over master branch last month: 23.69x speed increase over master branch since the beginning: 5.87x speed increase over master branch
date too much in the past it's just me going back to 2018-01-01
to check how slow it would be with such a wide range
The speed increase is definitely significant, but there was something bothering me. That's when I thought about caching.
What's the deal with caching in the context of analytics?
Caching of analytics in the master branch is done 1 time per each day in the date range, which can potentially result in tens or even hundreds of network call to the caching serves. Yes, caching helps because (if I'm not mistaken) DEV is using a globally distributed memcached pool of servers so the data is likely to be near you BUT going back and forth to the caching servers might be slower than asking the data to the DB directly if the range is big enough. Basically the round trips added to the caching server speed might result in more time spent than asking the web server to get the data from the DB (which is likely near the server) and send it back to the client.
I still think caching is important but caching each day separately might be counter productive. Since the data now it's loaded in bulk from the DB (GROUP BY
returns one row per each day in the same query), it might make sense to cache the whole result set instead of each row separately.
Step 3 - what happens if we remove all cache calls
user system total real
last week 2.369785 0.201472 2.571257 ( 3.973177)
last month 2.793212 0.148250 2.941462 ( 4.483640)
since the beginning 3.373822 0.158784 3.532606 ( 5.202688)
date too much in the past 5.493960 0.209581 5.703541 ( 7.480122)
last week: 11.48x speed increase over master branch last month: 43.52x speed increase over master branch since the beginning: 67.57x speed increase over master branch
As I suspected, the bigger the date range is, the more counter productive N cache calls are. Given that I'm on development with a totally different hardware and software than what's running the DEV production site times are going to be different and speed increases likely lower but it verifies my logic from the previous section.
Step 4 - add indexes
One thing that happens with analytics on many rows is that indexes start to matter (not so much in my development tests). Keep in mind that PostgreSQL can still decide not to use indexes if the data set is not big enough, but overall, they are a good idea.
What indexes? I took each query run by totals
and grouped_by_day
and checked WHERE
, GROUP BY
and FILTER
conditions. These are the fields I decided to add indexes on:
-
articles.published
(this is also going to benefit the entire website, since published articles is a super common query) comments.created_at
comments.score
follows.created_at
page_views.created_at
reactions.created_at
reactions.points
Since DEV has been in production for long and those tables contain lots and lots of data (I reckon reactions and page views especially), it's a good idea to (again :P) use the whole PostgreSQL power and add indexes concurrently. That means that PostgreSQL is going to use its concurrency capabilities to write the indexes in the background without stopping the normal write workflow to those tables. Indexes are normally written locking down an entire table, which can slow down the website during a migration. You can read more about all the details in the PostgreSQL doc about the subject.
So, what are the benchmarked timings?
user system total real
totals 1.678849 0.194811 1.873660 ( 2.974526)
last week 2.410151 0.109961 2.520112 ( 3.714314)
last month 4.619584 0.132775 4.752359 ( 6.109809)
since the beginning 7.117115 0.159913 7.277028 ( 8.774819)
date too much in the past 36.347774 0.402848 36.750622 ( 38.826439)
totals: 1.02x speed increase over no indexes version last week: 1.31x speed increase over no indexes version last month: 1.13x speed increase over no indexes version since the beginning: 1.06x speed increase over no indexes version
The reasons why they only slightly faster than the version with no indexes is that some indexes were already in place (those on the id/type columns and the user id) and that I don't have a big enough data set which, as previously said, can make PostgreSQL decide for a sequential scan or similar operations.
Step 5 - with indexes and no cache calls
Since the cache calls per each day were still in place, I tried without them:
user system total real
last week 1.870252 0.116198 1.986450 ( 3.161371)
last month 2.725406 0.144293 2.869699 ( 4.278060)
since the beginning 2.924799 0.146779 3.071578 ( 4.523648)
date too much in the past 5.456498 0.215746 5.672244 ( 7.229783)
again, with no cache calls the calculations are significantly faster, but the same evaluations made above are valid for this example.
last week: 1.29x speed increase over no cache and no indexes version last month: 1.02x speed increase over no cache and no indexes version since the beginning: 1.14x speed increase over no cache and no indexes version
Step 6 (and final step) - with indexes and one cache call
user system total real
last week 2.259850 0.134709 2.394559 ( 3.935191)
last month 3.120629 0.161101 3.281730 ( 4.906886)
since the beginning 3.539331 0.181215 3.720546 ( 5.575235)
date too much in the past 8.353001 0.302818 8.655819 ( 11.406922)
if compared with the example with indexes and one cache call per each day, the biggest difference can be seen when the date range becomes consistent: 3 months in the past is 1.95x faster and 1 year and a half in the past is 4.24x faster
If compared with the current master:
last week: 12.34x speed increase last month: 39x speed increase since the beginning: 64.12x speed increase
I hope this walkthrough was helpful
Related Tickets & Documents
Related to https://github.com/thepracticaldev/dev.to/issues/2311
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- @maestromac tracked down a nagging sign in issue and fixed it for people on browsers that don't entirely support web components. Thanks, Mac!
Fix js-breaking webcomponent #3087
What type of PR is this? (check all applicable)
- [x] Bug Fix
Description
It seems that having a standalone copy of webcomponent-loader.js
is both insufficient and breaking JS on some browser, particularly WaterFox. WebComponent's suggestion to use polyfill is in the follow manner:
<script src="node_modules/@webcomponents/webcomponentsjs/webcomponents-loader.js"></script>
but that also is not a viable option since we can't load dependency from node-module like that. The next best thing we can do is asynchronously load the polyfill from the CDN based this instruction. The README stated that the loader is efficient and will load only what is necessary.
I have tried to not rely on CDN by letting webpack dynamically import it but that also doesn't seem to work without additional webpack configuration. If there's a more viable solution or I missed something, please let me know.
Related Tickets & Documents
#3081 and quite possibly some users on https://github.com/thepracticaldev/dev.to/issues/2790
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
n/a
Added to documentation?
- [x] no documentation needed
- @maestromac also fixed inconsistent test, caused by a combination of time zone tests and the database time.
Fix inconsistent spec #3113
What type of PR is this? (check all applicable)
- [x] Bug Fix
Description
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [x] no documentation needed
- @wassimchegham added the DEV API link in the account settings page. Thanks, @wassimchegham!
docs: add DEV API link in the account settings UI #3119
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [ ] Bug Fix
- [x] Documentation Update
Description
Added the DEV API link (dev.to/api/) in the account settings UI.
Related Tickets & Documents
PR Closes https://github.com/thepracticaldev/dev.to/issues/3114
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
N/A
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
[optional] What gif best describes this PR or how it makes you feel?
Make `AdditionalContentBoxesController#randomize` private #3115
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Currently, AdditionalContentBoxesController#randomize
is a public
method and it does not make a lot of sense to leave it public
. This PR makes it private
as it is only used in AdditionalContentBoxesController
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
Refactor array creation #3116
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Refactors array creation in Api::V0::ArticlesController
and Credit
using Array.new
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
-
@arun also refactored the
Timeframer#datetime
method to use a hash instead of a conditional. Thanks again, @arun!
Refactor `Timeframer#datetime` to use a hash instead of a conditional #3117
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Replaces conditional in Timeframer#datetime
with a hash.
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- @mazentouati fixed the input's color for reading list filters in night theme. Thanks, @mazentouati!
Fix reading list filters input's color in night theme #3089
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
This fixes the input's color of the reading list filters in night theme ( or any future theme that does not have black as theme color )
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
Fix access token revoking #3131
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
I'm not 100% sure why but it seems that putting the ID as a keyword parameter for a DELETE
does not play nice with Rails. I've converted it to use an inline parameter following the same convention Rails uses for any other DELETE
/destroy action
Related Tickets & Documents
Closes #3079
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- I fixed an issue where a user's organization membership was not destroyed when their account was deleted.
Destroy organization memberships if user is destroyed #3125
What type of PR is this? (check all applicable)
- [x] Bug Fix
Description
Fixes a minor bug where a user's organization memberships were not destroyed when the user was destroyed.
Related #3077
- @mariocsee made the "Contact via Connect" option for listings checked by default. Thanks, Mario!
"Contact via Connect" checked by default #3128
What type of PR is this? (check all applicable)
- [x] Refactor
Description
Set default state of "Contact via Connect" to be checked (true) by default.
Related Tickets & Documents
resolves #3088
Added to documentation?
- [x] no documentation needed
Bug/Action Buttons Fix #3110
Bug Fix - Action Space
Buttons Edit and Manage | Add inline-flex to have action buttons always on same line
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [X] Bug Fix
- [ ] Documentation Update
Description
Fixes the Action Space Buttons on Mobile to display in the same line. Added display: inline-flex
to have the elements always maintain the same horizontal space.. causing them to wrap around together when there isn't enough space
Related Tickets & Documents
Fixes #3061
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?
-
@lightalloy moved calls to
SlackBot.delay.ping
toActiveJob
as part of the ongoing process to refactor ourDelayedJob
methods. Thanks, Anna!
Moved SlackBot.delay.ping calls to ActiveJob #3136 #3139
What type of PR is this? (check all applicable)
- [x] Refactor
Description
Moved SlackBot.delay.ping calls to a separate ActiveJob.
This may seem redundant compared with moving handle_asynchronous
methods to ActiveJob
s, but still I think it's worth doing because in such a way:
- we reduce dependency on the DelayedJob
- it's easier to debug in case of errors
Related Tickets & Documents
#3136
Extracts compound conditional to a private method #3144
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Extracts compound conditional in TagAdjustment#user_permissions
to a method.
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- @arun fixed a visual issue with night mode where an unread channel's name was in white and not black. Thanks again, @arun!
Fix visibility of unread channel name #3143
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] (Design) Bug Fix
- [ ] Documentation Update
Description
The channel with unread messages gets a yellow highlight but the text is hardly readable (See screenshot below). This PR fixes it by making the text black (#0a0a0a).
Before
After
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
-
@tannakartikey moved
DelayedJob
Mention
methods intoActiveJob
. Thanks, @tannakartikey!
Implement ActiveJob for Mention #3055
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Implemented ActiveJob for the following methods on Mention
:
Mention.create_all
Mention#send_email_notification
Related Tickets & Documents
#2497
Remove dead code #3146
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Removes UserStates
class as it is not used anywhere in the codebase. Also removes the tests exercising the UserStates
code.
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- @maestromac updated our service workers to display any errors in console. Thanks, Mac!
Update Service Worker and Favicon #3142
What type of PR is this? (check all applicable)
- [x] Misc
Description
If Service-Worker ever decides to serve 500.html
, an error will now be outputted to the browser.
Related Tickets & Documents
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Added to documentation?
- [x] no documentation needed
-
@lightalloy removed fetching podcasts from a callback and into an
ActiveJob
. Thanks, Anna!
Removed fetching podcasts from a callback #3098
What type of PR is this? (check all applicable)
- [x] Refactor
Description
- removed an
after_create
callback for fetching podcast episodes - added fetching podcast episodes to the admin controller action
- created request specs for podcast creating from
/admin/podcasts/new
- removed hacks that disable fetching podcasts while seeding and running tests
Creating a podcast and fetching episodes shouldn't be tightly coupled. We sometimes want to create a podcast without fetching episodes, e.g. for tests or for seeding the database.
Related Tickets & Documents
#2952
- @lightalloy added some much needed validations for podcasts. Thanks again, Anna!
Podcasts validations #3123
What type of PR is this? (check all applicable)
- [x] Bug Fix
Description
Implemented proper validations for podcasts and podcast episodes to avoid failing in the future.
- added model validations: fields presence,
slug
uniqueness,slug
uniqueness withusername
s and organizationslug
s - added db constraints
- added validation specs
Before merging need to check that all the podcasts
and podcast_episodes
records don't violate the provided constraints (non-null values and uniqueness)
Related Tickets & Documents
- @maestromac fixed a broken test. Thanks, Mac!
- @lightalloy updated the unique indices for notifications. Thansk, Anna!
Change notifications unique indexes #2525 #3012
What type of PR is this? (check all applicable)
- [x] Bug Fix
Description
- сhanged notifications indexes so that they take into account possible null values in
user_id
,organization_id
,action
fields Postgres allows to create records which contain the same fields if some of them contain null values, so I- created separate indexes for
user_id
andorganization_id
because notifications has eitheruser_id
ororganization_id
- made partial indexes for cases when
action
is null and when it's not null
- created separate indexes for
- added specs to check that notifications are not duplicated even when validations are skipped
Before merging, all the notifications that have duplicate fields %i[user_id organization_id notifiable_id notifiable_type action]
must be deleted
Related Tickets & Documents
#2525
- @lightalloy added validations for usernames and organization slugs against podcast slugs. Thanks again, Anna!
Validate usernames and organization slugs against podcast slugs #3148
What type of PR is this? (check all applicable)
- [x] Bug Fix
Description
- validate usernames and organization slugs not only against each other but also against podcast slugs
- added specs to test usernames and org slugs uniqueness and not being equal to reserved words
Podcast slug
s validation is in another pr #3123
-
@lightalloy also added an
organization_id
index to notifications. Thanks, Anna!
Added organization_id index to notifications #3155
What type of PR is this? (check all applicable)
- [x] Optimization
Description
Added organization_id
index to the notifications
table.
Explaining query without an index:
Notification.where("organization_id = 3 and read = false").select("count(*)").explain
------------------------------------------------------------------------
Aggregate (cost=780.32..780.33 rows=1 width=8)
-> Seq Scan on notifications (cost=0.00..775.94 rows=1753 width=0)
Filter: ((NOT read) AND (organization_id = 3))
(3 rows)
With the provided index:
Notification.where("organization_id = 3 and read = false").select("count(*)").explain
-----------------------------------------------------------------------------------------------------------------------
Aggregate (cost=338.36..338.37 rows=1 width=8)
-> Index Scan using index_notifications_on_organization_id on notifications (cost=0.28..333.98 rows=1753 width=0)
Index Cond: (organization_id = 3)
Filter: (NOT read)
(4 rows)
Fix analytics grouped by day with nil page views average #3159
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Description
I'm not sure how this is happening on production, but I guess there are page views that have time_tracked_in_seconds
that is nil for specific articles. If the average results in NULL
then the analytics page breaks.
This is a fix for that.
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
Remove old Dashboard Pro code #3161
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
There's a Dashboard::Pro
class that was lying around, I think it predates the AnalyticsService
@Zhao-Andy started. Since it's not used anywhere, I'm going to remove it
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
- @jrsam set the default reading time of 1 minute for articles within Liquid tags. Thanks, @jrsam!
Setting default reading time of 1 minute for articles within liquid tags #3084
resolves #3024
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [x] Bug Fix
- [ ] Documentation Update
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
-
@rhymes refactored a bit and made some performance improvements by removing a call for
.pluck
when it wasn't necessary. Thanks, @rhymes!
Performance: save some queries when pluck is not needed #3160
What type of PR is this? (check all applicable)
- [x] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] Documentation Update
Description
Rails pluck
forces an additional SQL query, such query is not always needed when the objects are already in memory or the requested ID is the primary key ID.
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [x] no documentation needed
Fix icons size in Safari reader mode #3157
What type of PR is this? (check all applicable)
- [ ] Refactor
- [ ] Feature
- [X] Bug Fix
- [ ] Documentation Update
Description
In Safari reader mode, icons become simply huge. I used the unused class which is added to icons only in reader mode: .reader-image-tiny
.
Related Tickets & Documents
I saw nothing about that.
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
The same on Apple smartphones
Added to documentation?
- [ ] docs.dev.to
- [ ] readme
- [X] no documentation needed
[optional] What gif best describes this PR or how it makes you feel?
(impatient given that it's my first contribution to this awesome project
New Issues and Discussions
- @andrewbrown requested a feature where there could be short links for articles. Thanks, @andrewbrown!
Short Links for Article URLs #3078
Is your feature request related to a problem? Please describe. Short Links.
Describe the solution you'd like Have an alternative link which is shorter than the full article link for sharing on platforms where characters are counted against you.
Describe alternatives you've considered
I can use short link tools but would feel there is more trust with the dev.to
url
Additional context
- @dance2die requested a feature where readers could suggest "related posts" for an article. Thanks, @dance2die!
Create a feature to suggest a "related post" #3083
Is your feature request related to a problem? Please describe. It's a brand-new feature request.
As I am reading posts, I see many posts that are related or more advanced topic of another posts.
As an example, first post below (Testing your apps like a boss...
) is an intro on how to test React components (using jest
& shows mocking) while the latter (A bit about Jest mocks
) is more of an in-depth post on "Jest mocks".
Describe the solution you'd like
This might require two features.
- Create a new "suggested/related post" at the bottom of the post above "Classic" & "Another post you might like".
- Add "suggest a related post" below "Suggest a Tweet".
Describe alternatives you've considered
Instead of creating a new category of "suggested/related post", we can display it as a "Another post you might like".
Additional context
May require a tag/moderator status for this feature.
- @nickytonline reported an issue where the image uploader button for comments jumps up once a file is selected. Thanks, @nickytonline!
Image uploader button on comment forms jumps up once a file is selected #3085
Describe the bug
The UI moves or covers other UI (in the case of mobile) when an image is selected to be added to a comment.
To Reproduce
- Go to any post and click on the image upload button of a comment.
- Select a file for upload.
- The file gets uploaded.
- The text input that contains the URL of the newly uploaded image appears causing the image upload button to move up (desktop) or covers other UI (mobile).
Expected behavior
The UI should not shift or cover other UI.
Screenshots
Desktop (please complete the following information):
- OS: macOS 10.14.5
- Browser: FireFox, Chrome, Safari
- Version: latest
Smartphone (please complete the following information):
- Device: iPhone 6 SE
- OS: iOS
- Browser: Brave
- Version: not sure
Additional context
- @georgecoldham opened a discussion about rate limiting new posts for users. Thanks, @georgecoldham!
Rate limiting new posts for users #3091
Is your feature request related to a problem? Please describe. I occasionally find my feed clogged with posts that I guess are bots? Or users trying to push users off site.
Describe the solution you'd like Rate limit users posts per hour. Perhaps loosen limits for people who have old enough accounts and/or enough post interactions.
Describe alternatives you've considered Creating a bot to filter out any similarly titled articles?
Additional context Example of my latest feed:
Examples of users who have been created today and have posted 15+ posts each within the last 40 minutes:
-
@mariocsee reported an issue where the footer is not properly centered in
/dashboard
within a certain width range. Thanks, Mario!
Non-centered footer in Articles Dashboard #3112
Describe the bug
In dev.to/dashboard
, the footer is aligned more to the right compared to the center column of articles. This happens between the widths of 950
and 1119
and it seems fine for anything wider or narrower.
To Reproduce
- Go to 'dev.to/dashboard'
- Adjust your width to between 950 and 1119
- Scroll down to the bottom
- See footer
Expected behavior
The footer should be centered in pages like /dashboard
where there are no cards to the right or left of the center column.
Desktop (please complete the following information):
- OS: MacOS
- Browser: Chrome
- Version: 75
Additional context Note that the footer is used all throughout DEV and is not always center aligned depending on what page you're on.
- @ksato1995 reported an issue where sometimes clicking a listing from the "Newest Listing" sidebar doesn't load a listing. Thanks, @ksato1995!
Sometimes doesn't load a listing #3118
Describe the bug
When I click a listing on newest listings
,
Sometimes I get a black screen like the screenshot below and I have to refresh the page to load the listing.
To Reproduce
Expected behavior
Load the clicked listing and show it on the screen. Screenshots
Screenshots are given above.
Desktop (please complete the following information):
- OS: Mac
- Browser: Chrome
- Version:Version 74.0.3729.169 (Official Build) (64-bit)
Smartphone (please complete the following information):
- Device:
- OS:
- Browser:
- Version:
Additional context
- @inozex reported an issue where dev.to in general was not working properly on the proprietary Facebook browser. Thanks, @inozex!
Dev.to stopped working on Facebook browser #3121
Describe the bug Dev.to pages won't load on Facebook built-in browser
To Reproduce Open a dev.to link from a Facebook share
Expected behavior Load the page correctly
Smartphone
- Device: nubia z17 lite (nx591j)
- OS: Android 7.1.1
- Browser: Facebook (Browser )
- Version: 224.0.0.33.114
- @rhymes requested a feature where the reading list had either pagination or infinite scrolling available. It's also a bit of a bug. Thanks, @rhymes!
Reading list: add pagination or infinite scroll #3122
Is your feature request related to a problem? Please describe.
Right now the reading list only displays the latest 64 items. If a user has more they can't access the other items until they've unsaved or archived "N - 64" articles.
See https://github.com/thepracticaldev/dev.to/blob/master/app/javascript/readingList/readingList.jsx#L24
Describe the solution you'd like
Either have a "Load more" button like listings have to load other items, or have infinite scrolling based pagination
- @rhymes Reading list: add number of items in the header
Reading list: add number of items in the header #3124
Is your feature request related to a problem? Please describe.
From the homepage it is possible to see how many items there are in the reading list. Once inside https://dev.to/readinglist that information is not accessible
Describe the solution you'd like
Add the number of articles in the title.
From the current
to something like
Additional context
This number has to update anytime an item is removed or archived I guess
- @jess requested a feature where you could use a hotkey to toggle the article preview. Thanks, Jess!
Hot key toggle for article preview #3129
Is your feature request related to a problem? Please describe. As a user, I'd like a hotkey that toggles the article preview when I'm writing a post so I don't have to manually click back and forth.
- @jess reported an issue where the reading time estimation was incorrect for an article. Thanks again, Jess!
Reading Time Estimation Incorrect for an Article #3130
@michael-tharrington commented on Tue Jun 04 2019
Describe the bug A particular post's (https://dev.to/backendandbbq/what-does-a-tech-lead-do-1cpj) estimated reading time is 1 minute, but the post is quite long and should be given a higher estimation.
To Reproduce
- Visit this post - https://dev.to/backendandbbq/what-does-a-tech-lead-do-1cpj
- Look at the reading time
- Look at the length of the article
Expected behavior I expect the reading time to be a clearer estimate. This post is much longer than a one-minute read.
Screenshots
Picture of email from reporter:
Additional context The reporter also referenced another issue (O minutes in link preview) that has already been reported - https://github.com/thepracticaldev/dev.to/issues/3024.
- @beatngu13 reported an issue where some math symbols and other symbols were not properly rendering. Thanks, @beatngu13!
Encoding issues #3133
Describe the bug
One of my articles uses various math symbols, which was fine when I wrote and published it. But today I've looked at it again and there are various encoding issues. Besides the math symbols, also plain dashes now cause issues, for instance:
[…] described situation – the GUI element identification problem […]
Which once was:
[…] described situation — the GUI element identification problem […]
You can check the expected behavior on the Medium version of the article.
To Reproduce
Go to https://dev.to/beatngu1101/a-formal-look-at-seleniums-nosuchelementexception.
Expected behavior
Compare with https://medium.com/@beatngu13/a-formal-look-at-seleniums-nosuchelementexception-bafce97df2e7.
Desktop (please complete the following information):
- OS: macOS Mojave (10.14.5)
- Browser: Firefox Quantum (67.0.1, 64 Bit)
- @michaeltharrington opened a discussion about providing the ability to opt out of displaying GitHub or Twitter icon publicly. Thanks, Michael!
Provide Ability to Opt Out of Displaying GitHub or Twitter icon on Profile/Name/Comments #3134
Is your feature request related to a problem? Please describe. A user wrote in asking if it was possible to opt out of having their GitHub link shown up next to their comments. They signed up via GitHub, but don't necessarily want their GitHub shown publicly. This option would give them more privacy.
Describe the solution you'd like We could add inputs specific to Twitter and GitHub under "Links" in the profile section of https://dev.to/settings. I'm thinking it'd be good to fill these in by default if the user signs up via one or both. However, the user could then easily visit their settings and remove what links are visible so that GH and Twitter icons could be removed.
Describe alternatives you've considered We could do the same as above but not fill in the GitHub/Twitter inputs by default. However, I think that generally these are quite nice to have, so I prefer the idea of filling them in by default.
-
@lightalloy wrote up a list for moving
DelayedJob
calls toActiveJob
. Contributors welcome! Thanks, Anna!
Move delay calls to ActiveJob #3136
As described in #2497, to be less dependent of DelayedJob
we also need to move jobs which are created by calling delay
method to ActiveJob
:
Here is the list of the calls:
- [x]
Slackbot.ping
- [x]
reaction.create
- [x]
ArticleAnalyticsFetcher.new.update_analytics
- [x]
HtmlVariantSuccess.create
- [x]
HtmlVariantTrial.create
- [x]
message.send_push
- [x]
RssReader.new.fetch_user
- [x] several calls in
trigger_delayed_index
- [x]
index.delay.delete_object("users-#{id}")
- [x]
user.follow
- [x]
chat_channel.delay.index!
- https://github.com/thepracticaldev/dev.to/pull/4317
Actions for each of the calls:
- create a corresponding
ActiveJob
, specify a queue name - call the required method inside a job
- replace the
delay
method calls with a jobperform_later
call - you may need to modify the existing tests by using
perform_enqueued_job
instead ofrun_background_jobs_immediately
helper
Remember to pass record id
s (if needed) instead of ActiveRecord objects to a job to avoid deserialization errors (#1621)
- @noahsscott requested a feature to more easily moderate users from their profile page. Thanks, @noahsscott!
Ability to moderate users from their profile page #3137
Is your feature request related to a problem? Please describe. With the recent occurrence of spam (see issue #3091) it would be useful to have the ability to flag or ban users from their profile page. This would save time marking individual posts if an account is responsible for more than one spam-like post and/or is clearly a spam account.
Describe the solution you'd like A button or icon to flag an account - visible to everyone, and another to ban - visible for moderators and above (depending on how people feel about moderators having this ability). Once marked, the user would choose a reason for why the account has been flagged or banned, with a field to provide further context for admins similar to the report page but perhaps wrapped in a modal? I can provide a mock-up if helpful.
Describe alternatives you've considered None, as the described actions of flagging or banning users are common-place among online communities.
Additional context After finding many spam posts on the latest feed I sent a list of the offending accounts to a dev.to admin, Michael, who pointed me to issue #3091 and the suggested improvements seem like they would prevent similar activity in the future. But in the case that spam accounts still get through it would save time if all users could flag accounts, and moderators could ban these accounts. I'm aware of the report page as I mentioned above, but having direct options of flagging or banning users from the profile page feels more appropriate given the actions that would be required anyway to moderate an account once it had started posting spam.
- @merri reported an issue wher the editor was removing meta data when viewing it to edit. Thanks, @merri!
The editor removes meta upon booting #3138
Describe the bug I've been working with one article for about a week now. Yesterday I added an organization and moved the article to the organization. Later on I noticed upon loading the article for editing that it was missing all the meta information. The editor only contains the article text. I can see the metadata flashing before the editor boots, but it disappears after boot (as buttons appear in bottom). In addition to this the editor shows New changes (clear) in the bottom all the time.
Also if I go to settings and change between v1 and v2 version of the editor I see no change happening to the editor. I never touched the setting before but the editor changed to something visually different on it's own a week or two ago.
So far I've noticed that if I open private (Firefox or Chrome) and login there the editor works as it should, so it also seems to be linked to session somehow.
To Reproduce
I have not seen the trouble to see if this duplicates, but these are roughly the steps that have happened during the history of writing the article:
- Login
- Create a new unpublished article under your own name (no organization)
- Edit the article text also on another machine or browser
- Edit the article in the first machine/browser: swap it to organization and also change some of the meta like the title
- Now each time you load the editor for this article it removes the meta
Expected behavior
Editor should never remove metadata for any reason.
Screenshots
Here I'm reloading the page and you can see how meta gets removed when the editor has booted:
Desktop (please complete the following information):
- OS: Windows 10
- Browser: Firefox
- Version: 67.0.1
- @ben requested a feature where there is a log of moderator actions. Thanks, Ben!
Moderators audit logs #3141
Moderators and admins take certain actions on the platform. Those actions can be discovered with certain breadcrumbs in our deeper app logs, the various things being created and destroyed, but we don't currently have a useful record of all actions taken.
I think it would be worth creating a table called mod_actions
or something like that. It would be a table that we append to any time a mod or admin does anything. For better overall auditing of what happened and when. It would be visible to other mods within a tag and for all admins to pay attention to.
I think others might have better insights on how to think about it, but I feel like these would be my high level feature thoughts:
- Who took the action (a user ID)
- What category (tag moderation, general community moderation, admin action, etc.)
- Info about that user's mod/admin roles at the time (which could change over time but this keeps it as a moment in time
- markdown to capture all info about the action, generated contextually to include relevant links etc.
- a dedicated name/slug for the type of action it was, e.g. tag removal, downvote, vomit vote, and all the other types of actions we might take. We could keep adding to this list as we go.
It should define an API to let us drop log lines in where relevant. They should execute with the lowest footprint possible, so they should execute asynchronously.
This would be a very helpful feature! I feel like it doesn't need a ton of existing knowledge of our codebase to implement in and of itself.
Feature Request: Block a user #3145
Is your feature request related to a problem? Please describe. It seems like if there is a way currently to block another user on the platform, it’s not intuitive. DEV is a happy place (especially when compared to the rest of the internet!) but there are still many risks and reason someone might want to block another user.
Describe the solution you'd like An easy-to-find button on a user’s profile that says “block.” Maybe there’s a form that requests feedback for the admin team about why there’s a block (maybe that person is being abusive outside the platform and the blocker just doesn’t want to see their activity on DEV, or maybe the blocker doesn’t want/need to give feedback)
Describe alternatives you've considered Contacting an admin to request a block could be one alternative, but that might make the blocker feel they’d have to explain or justify their request, making them less likely to request the block. Requiring that much “extra work” might also drive them away from requesting the block.
Additional context tl;dr: DEV.to is a happy place, but Bad Guys on the internet will follow us around anywhere they can find us, let’s help keep users safe and protect them from content or people they don’t want to see!
- @taillogs opened a discussion stating that we should have more information about the state of analytics. Thanks, @taillogs!
At a minimum, document the state of analytics #3151
Is your feature request related to a problem? Please describe. There are no official statements/pages/documents describing the state of analytics. I've read through the open issues and existing PR's and think I understand the current state, but many users are not going to find this info on Github.
Describe the solution you'd like I do not believe the analytics API is currently available. Regardless, whatever does/doesn't exist should be clearly documented in the frontend UX. I think most users (including myself) assume that a site of Dev.to scale would have analytics available (more than basic views, and reactions) and it's nearly impossible to find information about that.
Describe alternatives you've considered Impossible from user space.
Additional context I think basic information such as
- when your content was viewed
- region/languages it was viewed from
- avg read time
- % conversion
- user entry method (linked from Twitter, Google, Direct etc...)
- @taillogs reported an issue where editing content should be consistent throughout the site -- specifically, direct messages should be editable. Thanks again, @taillogs!
Editing content should be consistent throughout the site #3152
Is your feature request related to a problem? Please describe. Both posts and comments on posts are editable, but direct messages between users are not. Regardless of preference (twitter style of no editing or slack of edit everything), the UX should be consistent throughout the site.
Describe the solution you'd like Make direct messages editable (keep history if worried about abuse).
Describe alternatives you've considered Technically you could make comments uneditable because it would be consistent. Please don't do that though.
Consider adding a "bump" mechanism #3153
Is your feature request related to a problem? Please describe. After a post is published, it's often necessary to make drastic changes/corrections if something was missed in the initial editing process. Un-publishing the post is perfect for this, but when you re-publish the post it goes back to the exact same place in the "queue" (latest queue for example). Dev.to does not have a simple/clear discovery mechanism or visible persistency for content, this means the updated version of the post is unlikely to be seen.
Just yesterday I made a post with a bad title. 10 minutes later I realized, changed it and got 0 views or traction because it's window had passed. I ended up deleting, and then recreating the post which felt borderline abusive. Within minutes of reposting it ended up becoming my most well received post of all time. If I had not deleted it I would have lost out on that content.
I know it was partially discussed here https://github.com/thepracticaldev/dev.to/issues/3062. But that didn't feel entirely focused around the bump aspect.
Describe the solution you'd like Add a mechanism that allows you to bump/retweet/refresh existing content so it has another discovery window.
Describe alternatives you've considered Reverse bump (users positively receiving content causes it to move up the queue) might be possible, but not great if your content is already undiscoverable.
- @taillogs requested a feature to limit reaction count to be 1 per unique user. Thanks again, @taillogs!
Limit "reaction" count to be 1 per unique user #3154
Is your feature request related to a problem? Please describe.
Right now, users are able to react with 3 different emojis on each post. Although this is a fun way to interact with content, it's also a public rating mechanism for content. Anytime "points" are assigned to something, you need to worry about the way people will abuse/value those points. But because the reactions are so indirect in terms of what they represent to each user, nobody seems to use them consistently.
On the author side, this leads to a lack of visibility into how many users actually reacted to your post. You know the minimum (total reactions / 3) but not the true value.
On the poster side, if you're the type of person who gives only 1 reaction, you may perceive a post with 30 of each reaction as "Amazing". But in reality, that could have only been 10 people, not the 30 you expect.
This design would be less problematic for other platforms, but because Dev.to hides both follower counts and public page views, it leads to a potentially unhealthy feedback loop.
Describe the solution you'd like
Reader experience unchanged, they can react with 1-3 of the emojis. But externally, any number of reactions will be treated as 1 total reaction which increments the count on all of the emojis. That way, you can see the total number of unique reactions, as opposed to guessing.
Describe alternatives you've considered Always choose the reaction count with highest reactions.
Cover image and post info doesn't update on Twitter #3156
Describe the bug
If you publish a post with any cover image, then you edit it and change the cover image again and you try to tweet the article, Tweeter only shows the first cover image you set. It's the same for the title if you changed it after first publication.
To Reproduce
- Publish an article with a cover image
- Share it on tweeter
- Edit your post and change the cover image or the title
- Share it on Tweeter again (error is here for title and cover image)
Expected behavior
Tweeter must show the latest cover image
Screenshots
-
@timdeschryver reported an issue where the API does not properly accept the
top
parameter. Thanks, @timdeschryver!
bug(API): top parameter #3163
Describe the bug
The top parameter in the query string does not top the results.
To Reproduce
Without tag: https://dev.to/api/articles?top=2
With tag: https://dev.to/api/articles?tag=react&top=2
Something weird:
This does top the results https://dev.to/api/articles?tag=Angular&top=2
While this does not https://dev.to/api/articles?tag=Angular&top=1 https://dev.to/api/articles?tag=Angular&top=3
Expected behavior
I would expect that the results would be topped, for example top=2
should return 2 results.
Additional context
There are tests written on the API, but these aren't validating the length of the returned results https://github.com/thepracticaldev/dev.to/blob/master/spec/requests/api/v0/articles_spec.rb#L56
DEV-iOS
We haven't had much activity this week on the iOS repo. Feel free to make an issue, look at the codebase, or make a pull request!
DEV-Android
New Issues and Discussions
- @codingsam reported an issue where they opened the app and it took them to the offline page. Thanks, @codingsam!
Stuck on Offline page upon start #36
Describe the bug Two days ago, I tried to open the app and it just shows me the offline screen (see screenshot).
I have been using the app for more than a month without any issues. But since two days ago I always get this error. I can't do anything, like logout or something.
If I open Google Chrome and type "dev.to" I can use the web app without any problems. Only the apps returns me this error.
To Reproduce I am sorry, I hate to tell you this but I really don't know how to reproduce. I was just using it and one day I tried and it started to show me the error screen. From that day, it always does that and I can't do anything.
I am a software developer and I hate when people report a bug and tell me they don't know how to reproduce... But it is the case here.
Expected behavior Show the dev.to feed
Screenshots
Smartphone (please complete the following information):
- Device: Samsung Galaxy S7 Edge
- OS: Android
- Version 8.0.0
Additional context
I don't know what happened. Let me know what I can do to get more data to help you.
- @ryanwhocodes reported an issue where they got a strange error message. Thanks, @ryanwhocodes!
Error URL does not exist when trying to save articles #37
Describe the bug A clear and concise description of what the bug is.
I keep getting the error about URL does not exist when trying to save posts - please see attached screenshot. It was using the DEV app on Android.
To Reproduce Steps to reproduce the behavior: Log into account via mobile app. Click on edit for one of your articles Click on save The error appeared
NOTE! This error only was raised when trying to save some articles but not all of them so I cannot tell what the cause is or how to reliably reproduce the issue.
Expected behavior A clear and concise description of what you expected to happen.
You should click on save and it doesn't show the error and it successfully updated the article.
Screenshots If applicable, add screenshots to help explain your problem.
Smartphone (please complete the following information):
- Device: Samsung A50
- OS: Android 9
- Version 9
Additional context Add any other context about the problem here.
That's it for this week! Stay tuned next week's edition.
Top comments (1)
Thank you for your contributions @rhymes , @ben , @danielcompton, @maestromac , @wassimchegham , @arun , @mazentouati , @mariocsee , @bdlb77 , @lightalloy , @tannakartikey , @jrsam, @max , @andrewbrown , @dance2die , @nickytonline , @georgecoldham , @ksato1995 , @inozex , @jess , @beatngu1101 , @michaeltharrington , @noahsscott , @merri , @desi , @taillogs , @timdeschryver , @codingsam , @ryanwhocodes !