Christina Gorton for The DEV Team

Posted on Oct 26, 2020

Hacktoberfest Week Five — Thank you to our Contributors on Forem ❤️

#opensource #gratitude #meta #hacktoberfest

It’s the fifth week of Hacktoberfest and we are continuing to highlight the amazing people in our community who have contributed to Forem's codebase!

We’re thrilled to share that we’ve had 10 contributors this week commit improvements across the many projects we have in our web, iOS, and Android apps 🎉

Thanks to the help of these wonderful folks, we’ve merged 15 PRs this past week. Forem is still a small team so this amount of feature-building and bug-squashing is truly only possible because of the community. As Forem grows, we intend to continue enabling the open source ecosystem to improve and expand our offerings with their invaluable help. We appreciate everyone for helping us sow this open source commitment from day one.

If you are interested in contributing, check out our post on Forem projects you can contribute to this Hacktoberfest:

Contribute to Forem this Hacktoberfest!

Christina Gorton for The DEV Team ・ Oct 2 '20

#opensource #meta #hacktoberfest #contributorswanted

In no particular order, here are the folks who made commits this week, their GitHub profiles, and their merged PRs.

Takuma

Fix reading list counter #10763

takmar posted on Oct 10, 2020

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

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

Description

I found that a wrong reading list count was caused by caching. The cache of cached_reading_list_article_ids is changed only when public_reactions_count is updated, but we must also recache it when user archives and unarchives an article.

To achieve this, I added the last_changed_archive_at column to the users table and cache keys. I could not come up with an idea to fix this without adding an extra column. Will you let me know If you have better thoughts, I'll give it a try.

Related Tickets & Documents

Closes https://github.com/forem/forem/issues/9770

QA Instructions, Screenshots, Recordings

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

Added tests?

[x] yes
[ ] no, because they aren't needed
[ ] no, because I need help

Added to documentation?

[ ] docs.forem.com
[ ] readme
[x] no documentation needed

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

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

View on GitHub

Sam

Fix grammar error in Code Climate #10935

sharkham posted on Oct 19, 2020

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

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

Description

Combined two sentences in the last paragraph of Code Climate to fix sentence fragments.

Added tests?

[ ] yes
[x] no, because they aren't needed
[ ] no, because I need help

Added to documentation?

[x] docs.forem.com
[ ] readme
[ ] no documentation needed

View on GitHub

Simon Fish

Document overriding mailer templates #10574

boardfish posted on Oct 04, 2020

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

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

Description

Updates documentation relating to emails. In particular, documents how the mailer views can be overridden, especially for devise_invitable.

While working on this, I identified that there aren't mailer previews for devise_invitable (#10575).

Related Tickets & Documents

#9849

QA Instructions, Screenshots, Recordings

Edit the contents of a mailer view under the given directories, and check the mailer previews to make sure they make sense.

Added tests?

[ ] yes
[x] no, because they aren't needed
[ ] no, because I need help

Added to documentation?

[x] docs.forem.com
[ ] readme
[ ] no documentation needed

View on GitHub

Rafi

Adding file extensions to gitattributes #10945

Rafi993 posted on Oct 20, 2020

What type of PR is this?

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

Description

Adding file extensions to .gitattributes for better diff. It is mostly inspired from https://tekin.co.uk/2020/10/better-git-diff-output-for-ruby-python-elixir-and-more

Related Tickets & Documents

Closes https://github.com/forem/forem/issues/10946

QA Instructions, Screenshots, Recordings

Try doing git diff using git cli before and after this commit on a ruby file

Added tests?

[ ] yes
[x] no, because they aren't needed
[ ] no, because I need help

Added to documentation?

[ ] docs.forem.com
[ ] readme
[x] no documentation needed

View on GitHub

Removing hard-coded email #10125

Rafi993 posted on Sep 01, 2020

What type of PR is this?

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

Description

Removing hardcoded email yo@dev.to

Related Tickets & Documents

Closes https://github.com/forem/forem/issues/9826

Added tests?

[x] yes
[ ] no, because they aren't needed
[ ] no, because I need help

Added to documentation?

[ ] docs.forem.com
[ ] readme
[x] no documentation needed

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

View on GitHub

Tudor Pavel

Avoid CSS word wrap in video player duration #10889

tudorpavel posted on Oct 17, 2020

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

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

Description

These new CSS rules override the overflow-wrap: anywhere and word-break: break-word rules in .crayons-article__header and also the overflow-wrap: anywhere rule in .crayons-card, but only for the JW player part of the DOM.

Related Tickets & Documents

Closes #10799

QA Instructions, Screenshots, Recordings

Before:

After:

Added tests?

[ ] yes
[x] no, because they aren't needed
[ ] no, because I need help

Added to documentation?

[ ] docs.forem.com
[ ] readme
[x] no documentation needed

View on GitHub

Preserve newlines and indentation in <pre> tags from RSS import #10922

tudorpavel posted on Oct 19, 2020

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

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

Description

I tracked down the issue to the reverse_markdown gem and found this unmerged fix: https://github.com/xijo/reverse_markdown/pull/78. Since it seems we are already overriding the <pre> converter I went ahead and added the fix to our CustomPre implementation, together with a simple spec.

Related Tickets & Documents

Closes #10104

QA Instructions, Screenshots, Recordings

I tested using the sample RSS feed provided in the issue.

The easiest way I found for me to test the feed parsing was to create a draft Article and then from rails console perform an update with the code from RssReader:

feed = Feedjira.parse(File.read('sample_rss.xml'))

Article.last.update(
  body_markdown: RssReader::Assembler.call(feed.entries.first, User.find(4), feed, feed.entries.first.url.strip.split("?source=")[0])
)

Added tests?

[x] yes
[ ] no, because they aren't needed
[ ] no, because I need help

Added to documentation?

[ ] docs.forem.com
[ ] readme
[x] no documentation needed

View on GitHub

Fix RSpec unit testing when using Docker dev setup #10950

tudorpavel posted on Oct 20, 2020

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

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

Description

When using the Docker dev setup, Elasticsearch is available at elasticsearch:9200 instead of localhost:9200 so the VCR testing config was not ignoring the requests needed for RSpec setup when doing Search::Cluster.recreate_indexes in spec/rails_helper.rb. Also the WebMock setup allows localhost requests which should also include Elasticsearch requests.

This seems okay to me, but I'm not sure if there is a better solution. One issue I might have is that a detail related to the Docker dev setup is bleeding into common RSpec configs. What do you think, @citizen428?

Related Tickets & Documents

Closes #10579

QA Instructions, Screenshots, Recordings

With this change you can run unit tests when using the Docker dev setup, for example this spec was failing at Search::Cluster.recreate_indexes in spec/rails_helper.rb and is now passing.

bundle exec rspec spec/liquid_tags/vimeo_tag_spec.rb

Added tests?

[ ] yes
[x] no, because they aren't needed
[ ] no, because I need help

Added to documentation?

[ ] docs.forem.com
[ ] readme
[x] no documentation needed

View on GitHub

Use matched substring in Spotify liquid tag to avoid trailing whitespace #10953

tudorpavel posted on Oct 20, 2020

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

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

Description

The bug was caused by a trailing whitespace character in the embed URL. Using the matched substring from MatchData avoids any additional whitespace. Also updated the spec to use explicit values instead of duplicating the logic from the code.

Related Tickets & Documents

Closes #10951

QA Instructions, Screenshots, Recordings

Adding a Spotify liquid tag to an Article will now result in a valid embed iframe, while before it would display the text "Bad Request".

For example:

{% spotify spotify:track:3GfOAdcoc3X5GPiiXmpBjK %}

Before:

After:

Added tests?

[x] yes
[ ] no, because they aren't needed
[ ] no, because I need help

Added to documentation?

[ ] docs.forem.com
[ ] readme
[x] no documentation needed

View on GitHub

Anks

Add aliases for mod_roundrobin_notifications + email_follower_notifications to user model #6892 #10892

matisnape posted on Oct 17, 2020

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

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

Description

Related Tickets & Documents

Closes #6892

QA Instructions, Screenshots, Recordings

n/a

Added tests?

[x] no, because they aren't needed

Added to documentation?

[x] no documentation needed

View on GitHub

PrimeNoodles

Mod Center: clicking on article title opens a in new tab #9314

PrimeNoodles posted on Jul 14, 2020

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

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

Description

Was able to create a clickable title on the modCenter page where clicking on the article title would open a new tab to the the article. Clicking on the label will bring up the original inline article mod.

Related Tickets & Documents

Closes https://github.com/forem/forem/issues/8976

I was also able to get a clickable link on the author to work, which seemed to have no errors on my local host end. However, when trying to commit and run the test cases on singleArticle.text.jsx, it wasn't able to pass one of the test on line 73 to 77 where it renders the author's name. I have commented the code out if anyone can look and see what the problem is.

I also wasn't able to implement @jacobherrington suggestion of having a external link icon simply because I wasn't able to import the externalicon.svg, maybe it's connect to the crayon system?

QA Instructions, Screenshots, Recordings

Added tests?

[x] yes
[ ] no, because they aren't needed
[ ] no, because I need help

Added to documentation?

[ ] docs.dev.to
[ ] readme
[x] no documentation needed

View on GitHub

Andrew Bone

refactor: Use keyboard shortcut hook in article form #11014

Link2Twenty posted on Oct 22, 2020

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

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

Description

Now we have a helper for keyboard shortcuts we can move old shortcuts over to this new method

Related Tickets & Documents

#10878

QA Instructions, Screenshots, Recordings

Press ctrl/command + shift + p in the new post page it should toggle the preview

Added tests?

[ ] yes
[x] no, because they aren't needed
[ ] no, because I need help

Added to documentation?

[ ] docs.forem.com
[ ] readme
[x] no documentation needed

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

N/A

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

View on GitHub

Allow keyboard shortcuts #10713

Link2Twenty posted on Oct 07, 2020

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

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

Description

Add handler for keyboard shortcuts. This allows custom shortcuts to be set up on a page by page (or even component by component) basis.

React demo

Usage

In this example we're going to run a function when the user presses CTRL+SHIFT+P

const shortcuts = {
  "ctrl+shift+KeyP":  (e) => {
    e.preventDefault();
    someFunction();
  }
}

<KeyboardShortcuts shortcuts={shortcuts} />

Related Tickets & Documents

related: #5023 #596

QA Instructions, Screenshots, Recordings

This facilitates changes in the future

Added tests?

[x] yes
[ ] no, because they aren't needed
[ ] no, because I need help

Added to documentation?

[ ] docs.forem.com
[ ] readme
[x] no documentation needed

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

Once this is merged I'd like to modify this current shortcut to use this method before thinking about what other shortcuts could/should be added.

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

View on GitHub

Robin Gagnon

refactor: Use keyboard shortcut hook in listing modal #11017

reobin posted on Oct 22, 2020

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

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

Description

Use the new keyboard shortcut hook to listen for Escape keypress to close an open listing modal

Related Tickets & Documents

#10878

QA Instructions, Screenshots, Recordings

Go to listings
Open a listing
Press Escape
Verify that the listing modal closes

Added tests?

[ ] yes
[ ] no, because they aren't needed
[x] no, because I need help

I'd like to add tests for this, but the <Listings /> component does not currently have tests.

It seems to need a lot of stuff in order to set it up (mocking, adding divs to the DOM, etc.).

Any help with that is appreciated.

Closing the modal is probably not the most crucial test to do in this component, but if making the set up opens the door to more tests, then it's a win.