Skip to content

AO3-2312 admin post kudos#3082

Closed
tuff wants to merge 9 commits into
otwcode:masterfrom
tuff:AO3-2312_admin_post_kudos
Closed

AO3-2312 admin post kudos#3082
tuff wants to merge 9 commits into
otwcode:masterfrom
tuff:AO3-2312_admin_post_kudos

Conversation

@tuff

@tuff tuff commented Sep 26, 2017

Copy link
Copy Markdown
Contributor

Issue

https://otwarchive.atlassian.net/browse/AO3-2312

Purpose

Lets guests and users leave kudos on admin posts.

Testing

  • While logged in as a user, try to leave kudos on an admin post from that post's page. Check that the success flash message appears and your kudos is listed.
  • Try it as a guest.
  • Try to leave kudos multiple times. Check that the "you already left kudos" error message appears.
  • Try the above steps with JS off.

Note: I found that a kudo left while logged in as a admin counts as having been left by a guest. I assume that is expected as admins are not users. Let me know in review if that's not what's desired.

Comment thread app/models/kudo.rb Outdated
Comment thread app/models/kudo.rb Outdated
Comment thread app/models/kudo.rb Outdated
Comment thread app/models/kudo.rb Outdated
@tuff
tuff force-pushed the AO3-2312_admin_post_kudos branch from 4d3f84b to 8c8e3fa Compare September 26, 2017 20:26

@ariana-paris ariana-paris left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd like someone from front-end to okay the class names and CSS changes, but I made some comments about the tests and aside from that, it looks good to me.

Comment thread features/comments_and_kudos/kudos.feature Outdated
Comment thread features/comments_and_kudos/kudos.feature Outdated
@otw-deploy otw-deploy closed this Oct 31, 2017
@zz9pzza zz9pzza reopened this Oct 31, 2017
@tuff
tuff force-pushed the AO3-2312_admin_post_kudos branch from 14410f0 to b56fdf1 Compare December 2, 2017 20:20
Comment thread app/models/kudo.rb Outdated
Comment thread app/models/kudo.rb Outdated
@tuff
tuff force-pushed the AO3-2312_admin_post_kudos branch from c8c0b9e to 660b74a Compare December 2, 2017 20:37
@tuff
tuff force-pushed the AO3-2312_admin_post_kudos branch 2 times, most recently from 6703061 to a280e6a Compare July 13, 2018 23:17
@zz9pzza

zz9pzza commented Sep 28, 2022

Copy link
Copy Markdown
Contributor

So this has lots of merge conflicts.

And now Kudo is given by a users rather than psueds.

@sarken

sarken commented Sep 30, 2022

Copy link
Copy Markdown
Collaborator

The kudos button should also only be available on an individual admin post's show page, like the comment form is only available there.

@tuff

tuff commented Oct 11, 2022

Copy link
Copy Markdown
Contributor Author

@zz9pzza @sarken I've fixed this up and removed the kudos button from the admin posts index.

Given /^I have posted an admin post$/ do
step(%{I am logged in as a "communications" admin})
step("I make an admin post")
step("I am on the homepage")

@tuff tuff Oct 11, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Without this, the JS-enabled tests fail as we land on example.com after making the admin post. I don't know why that is...is it related to this default URL value? I can't repro manually 😕

Please suggest a better fix if you know one!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's been a while, but there is a better fix now:
In AdminPostsController#create change the redirect_to(@admin_post) to redirect_to admin_post_path(@admin_post).

Explanation:
The controller calls redirect_to on a model object (instead of a path). When you call redirect_to on a model, Rails automatically tries to convert it to a full URL, instead of a path, and when the host is set to example.com, that means that it tries to redirect to example.com.

Comment on lines +190 to +191
And I follow "Default Admin Post"
And I press "Kudos ♥"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If master is merged into this branch (which would be nice), then this test needs an update:

Suggested change
And I follow "Default Admin Post"
And I press "Kudos ♥"
And I follow "Default Admin Post"
And all emails have been delivered
And I press "Kudos ♥"

So the email from creating the admin wont mess up the test.

@Bilka2

Bilka2 commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Adopted at #5931

@Bilka2 Bilka2 closed this Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants