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.

Sign up for GitHub

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

Jump to bottom

issue 956 - exclude orphaned stories #971

Merged
merged 8 commits into from
Dec 11, 2023
Merged

issue 956 - exclude orphaned stories #971

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
87c78c1
issue 956 - exclude orphaned stories from both dashboard and api cont…
ice1080 Oct 5, 2023
53a6962
issue 956 - exclude orphaned stories from home controller for map
ice1080 Oct 5, 2023
b1934f0
issue 956 - undoing removal of orphaned stories in StoriesPage
ice1080 Oct 6, 2023
927a1af
issue 956 - unit test
ice1080 Oct 6, 2023
bcd964d
issue 956 - unit test
ice1080 Oct 6, 2023
5dd7b82
issue 956 - unit test - updating comments and variable names for clarity
ice1080 Oct 8, 2023
dfcdcfc
changes per comments
ice1080 Nov 14, 2023
a4ae2a2
Remove unused let
lauramosher Dec 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion rails/app/controllers/home_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ def create
end

helper_method def stories
policy_scope(current_community.stories)
community_stories = policy_scope(current_community.stories)
community_stories = community_stories.joins(:speakers).distinct
community_stories = community_stories.joins(:places).distinct
community_stories
end
end
1 change: 0 additions & 1 deletion rails/spec/requests/api/public_stories_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@

get "/api/communities/cool_community/stories"


expect(json_meta["total"]).to eq(1)
expect(json_response["stories"].map { |s| s["id"] }).to contain_exactly(story_1.id)
end
Expand Down
42 changes: 42 additions & 0 deletions rails/spec/requests/home_request_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,47 @@
expect(response).to redirect_to("/users/sign_in")
end
end

context "orphaned stories" do
let(:community) { FactoryBot.create(:community, name: "Orphaned Community") }
let(:user) { FactoryBot.create(:user, community: community) }
let!(:place_1) { create(:place, community: community, region: "test place 1") }
let!(:place_2) { create(:place, community: community, region: "test place 2") }
let!(:speaker_1) { create(:speaker, community: community) }
let!(:speaker_2) { create(:speaker, community: community) }
let!(:story_1) do
create(
:story,
community: community,
places: [place_1],
speakers: [speaker_1],
desc: "This story should not be visible because it has no speaker",
permission_level: :anonymous
)
end
let!(:story_2) do
create(
:story,
community: community,
places: [place_2],
speakers: [speaker_2],
desc: "This story should not be visible because it has no place",
permission_level: :anonymous
)
end

before do
login_as user
end

it "excludes stories with no speaker and/or place" do
speaker_1.destroy
place_2.destroy
get "/home"

expect(response.body).not_to include('This story should not be visible because it has no speaker')
expect(response.body).not_to include('This story should not be visible because it has no place')
Comment on lines +61 to +62
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works; however, if you wanted to make use of the json_response helper, we can set this test up similar to the one you had in the api spec:

expect(json_response["stories"].length).to eq(0)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @lauramosher, this test is actually not an api test, so the json_response doesn't parse the body since it's html instead of json. Let me know what you'd prefer, Rudo had asked me to create that test so that you had tests on the page contents instead of just the api tests.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see where I misunderstood. I noted that the json_response helper was included in this file and assumed we were testing a JSON endpoint.

Rudo is correct in that we need a test for this request as it's the one that had the issue. I believe Rudo was ensuring that we tested both the public API and the internal Home map story results since they are powered by two separate controllers.

The home request is powered by a partial that renders JSON (_home.json.jbuilder) and is completely separate from the public API endpoints available in /api/.

Like I said, this works, so I'm not super worried about changing it. If you're interested/wanted to, we could use rspec's type: :view attribute and test that this request renders the _home partial and we would then be able to assert on the JSON instead of grepping through HTML blobs.

end
end
end
end