Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The Series list is showing empty series (with 0 articles) #9658

Closed
beeman opened this issue Aug 6, 2020 · 15 comments · Fixed by #11227
Closed

The Series list is showing empty series (with 0 articles) #9658

beeman opened this issue Aug 6, 2020 · 15 comments · Fixed by #11227
Assignees
Labels
bug always open for contribution

Comments

@beeman
Copy link

beeman commented Aug 6, 2020

Today I saw there is a page that shows the users' series. While this is awesome, I think it would be even more awesome if it would not show the empty articles.

In my case, as seen in the screenshot, I have a few series that are there as work-in-progress, I'd love those not to be visible until they have at least one post published. In another case, a series appears double. Probably because I recreated some of the posts.

To Reproduce

Expected behavior

I would love it if the empty series (0 articles) would not appear, and add an option for authors to delete series.

Screenshots

image

Desktop (please complete the following information):

  • OS: macOS Catalina 10.15.5 (19F101)
  • Browser: Brave
  • Version: Version 1.11.104 Chromium: 84.0.4147.105 (Official Build) (64-bit)

Smartphone (please complete the following information):

  • Device:
  • OS:
  • Browser:
  • Version:

Additional context

I'm happy to give a go at implementing this, it would be great if someone could give me some pointers on what changes are expected :-)

@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2020

Thanks for the issue! We'll take your request into consideration and follow up if we decide to tackle this issue.

To our amazing contributors: issues labeled type: bug are always up for grabs, but for feature requests, please wait until we add a ready for dev before starting to work on it.

To claim an issue to work on, please leave a comment. If you've claimed the issue and need help, please ping @forem/oss and we will follow up within 3 business days.

For full info on how to contribute, please check out our contributors guide.

@citizen428 citizen428 added area: series bug always open for contribution labels Aug 7, 2020
@rhymes
Copy link
Contributor

rhymes commented Aug 7, 2020

@beeman go ahead! I think we can start by only showing series that have at least one article in the public series page.

You can find the code that selects series here: https://github.com/forem/forem/blob/master/app/controllers/collections_controller.rb and its tests here https://github.com/forem/forem/blob/master/spec/requests/collections_spec.rb (maybe you can add a test to make sure that series without articles don't appear in the result).

I would deal with "deleting empty series" separately to keep the PR small

@rhymes
Copy link
Contributor

rhymes commented Sep 16, 2020

@beeman are you still working on this? Let us know, thanks!

@beeman
Copy link
Author

beeman commented Sep 22, 2020

@rhymes I started on it but got distracted. Happy for someone else to pick it up.

If I decide to look at it again, I'll post a message here first.

@citizen428
Copy link
Contributor

I unassigned you now @beeman, thanks for giving it a shot 😃

@cwray-tech
Copy link
Contributor

Hey, @citizen428 I would really like this issue if possible. Also, I would like to add edit and delete functionality to the series. Should I do this all in the same pr, or is there another issue I should tackle as well? Thanks!

@rhymes
Copy link
Contributor

rhymes commented Sep 28, 2020

@cwray-tech I think they should be in different PRs are they are related to different functionalities. I'm assigning you this issue

@cwray-tech
Copy link
Contributor

@rhymes thank you! I will tackle this first then. Also, thanks for assigning this to me.

@rhymes rhymes added external contributors welcome contribution is welcome! and removed external contributors welcome contribution is welcome! labels Oct 1, 2020
@rhymes
Copy link
Contributor

rhymes commented Oct 1, 2020

@cwray-tech for the "managing" part of a series you can refer to this issue here #9008

@cwray-tech
Copy link
Contributor

@rhymes why don’t the series have their own model and controller?

@rhymes
Copy link
Contributor

rhymes commented Oct 1, 2020

@cwray-tech they do, unfortunately the domain name is different. They are called collections in the code: https://github.com/forem/forem/blob/master/app/models/collection.rb and https://github.com/forem/forem/blob/master/app/controllers/collections_controller.rb

@cwray-tech
Copy link
Contributor

@rhymes ah!! This helps so much. Thank you 😊

@cwray-tech
Copy link
Contributor

Hi @citizen428 or @rhymes I am pretty inexperienced with Ruby on Rails syntax. What I am trying to do is add a method in the Collection model called has_articles? then use that method in the index controller method to only get collections with articles, but I just cannot figure this out! Maybe you can help?

Here is my Collection model method has_articles? :

def has_articles?
    articles.empty ? false : true
end

In the index method of the CollectionsController, I want to filter the collection items by whether the collection has articles, and I have tried this a variety of ways. Here is one:

def index
    @user = User.find_by!(username: params[:username])
    @collections = @user.collections.where.has_articles?.order(created_at: :desc)
end

What am I doing wrong here? Would love your help if you have the time!

@citizen428
Copy link
Contributor

citizen428 commented Nov 2, 2020

Hi @cwray-tech, thanks for giving this a shot.

Here's some feedback:

  1. In Ruby, predicate methods names generally end with ?, so the method name is empty?, not empty.
  2. You don't need a ternary here, the predicate already returns true or false.
  3. In Rails vs. pure Ruby it's often more common to use present? instead of empty?, but you can also use articles.any? here, which is semantically clear and basically renders the has_articles? method a bit obsolete.

Now on to the index method:

You can not just chain a normal method onto a where, you need to use a Rails scope for this (that includes the where). You can find the relevant documentation here.

All that said, what you want to achieve can already be done with standard Rails, no need to add any new methods:

@collections = @user.collections.joins(:articles).order(created_at: :desc)

This will create SQL like this:

SELECT "collections".* FROM "collections" INNER JOIN "articles" ON "articles"."collection_id" = "collections"."id" WHERE "collections"."user_id" = 12 ORDER BY "collections"."created_at" DESC

Since joins generates an inner join, collections without articles won't be returned:

[17] forem(main)> @user.collections.count
   (2.2ms)  SELECT COUNT(*) FROM "collections" WHERE "collections"."user_id" = $1  [["user_id", 12]]
1
[18] forem(main)> @user.collections.joins(:articles).count
   (18.3ms)  SELECT COUNT(*) FROM "collections" INNER JOIN "articles" ON "articles"."collection_id" = "collections"."id" WHERE "collections"."user_id" = $1  [["user_id", 12]]
0

Hope this helps, please let me know if you have any more questions!

@cwray-tech
Copy link
Contributor

cwray-tech commented Nov 2, 2020 via email

rhymes pushed a commit that referenced this issue Nov 3, 2020
)

* added join to index collections controller

* + collections test to not show empty series
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug always open for contribution
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants