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

Error in listings #13054

Open
DevLorenz0 opened this issue Mar 18, 2021 · 5 comments
Open

Error in listings #13054

DevLorenz0 opened this issue Mar 18, 2021 · 5 comments
Labels
bug always open for contribution internal team only internal tasks only for Forem team members size: estimated XS less than a few days to complete tech: frontend changes will primarily involve frontend technologies

Comments

@DevLorenz0
Copy link

Describe the bug

Two problems with listings. The second is caused by the first. Their not really bugs, just problems with the UX / UI.
When I bump my listing, nothing say me that it works, so I end thinking that it doesn't work. Then, especially because the user doesn't receive feedback, he can be tempted to click another time the bump button (as I did...). And he lost another credit for every click, even if he gains nothing from that additional bump.

To Reproduce

Go to "listings"
Create a listing
Go on "edit" (the listing)
Bump
--> Click the bump button - It does not update instantly
--> Click two or more time the bump button - You lose credits for nothing

Expected behavior

An alert / small message that says: "Your request has been approved, your list will be bumped in the following minutes" or something like that next to the bump button should be good. Then, especially because the user doesn't receive feedback, he can be tempted to click another time the bump button (as I did...). So I think it's important to add a limit for bumping, maybe one every 15 minutes. Although for security / best user experience reasons (for the whole community) it would be better to limit the creation of list/bumping of lists to 24 hours.

Screenshots

image

Desktop (please complete the following information):

OS, version: Microsoft, Windows 10
Browser, version: Chrome, latest (88.0.4324.190)

Smartphone (please complete the following information):

Not specific to smartphones.

Additional context

I discovered the bug and just don't want others to have the same problem.

Regards

@github-actions
Copy link
Contributor

Thanks for the issue, we will take it into consideration! Our team of engineers is busy working on many types of features, please give us time to get back to you.

Feature requests that require more discussion may be closed. Read more about our feature request process on forem.dev.

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. The OSS Community Manager or the engineers on OSS rotation will follow up.

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

@cmgorton
Copy link
Contributor

Thanks for letting us know! I use listings a lot so this is good information. I'll let our engineers know and see if we can add some user feedback once the bump listing button is clicked.

@cmgorton cmgorton added area: listings bug always open for contribution labels Mar 18, 2021
@mstruve mstruve added tech: frontend changes will primarily involve frontend technologies size: estimated XS less than a few days to complete internal team only internal tasks only for Forem team members labels Apr 8, 2021
@cmgorton
Copy link
Contributor

cmgorton commented Apr 8, 2021

Here is a video of what you see when you hit the bump button. Right not there is no indication at all the the button was hit or that it has bumped your listing.
I could confirm when I went to the listings page that the listing had been bumped but again on that actual page with the button to bump there was no indication that I had. I could also keep clicking the button which results in multiple credits being spent.

Screen.Recording.2021-04-08.at.11.41.52.AM.mov

@mstruve mstruve added this to Backend Focused in Current Cycle Work Apr 8, 2021
@mstruve mstruve moved this from Backend Focused to Frontend Focused in Current Cycle Work Apr 8, 2021
@nickytonline nickytonline self-assigned this Apr 22, 2021
@nickytonline nickytonline moved this from Frontend Focused to In Progress 🖥 in Current Cycle Work Apr 22, 2021
@nickytonline
Copy link
Contributor

nickytonline commented Apr 22, 2021

I was trying to figure out how this was working and then I discovered, it's a form submit, but with no navigation away from the page, i.e. an HTTP 204. I was confused as it looked like a regular form submit, but it appeared to have AJAX enabled.

We'll need to change that up to provide feedback to the user whether or not the bump was successful.

image

I looked up how a response in rails sends back an HTTP 204. Apparently you use head :no_content. My thought is to do this request via AJAX or have the bump submit return 200 and refresh the page with a success/fail message. Looking through the code though, there does not appear to be any head :no_content set for listings. I checked the controller and model. Still digging.

image

@rhymes
Copy link
Contributor

rhymes commented Apr 23, 2021

@nickytonline unfortunately it's a weird corner case bug with some code that's hard to decipher

I found it perusing the logs after clicking on "Bump", that 204 is not intentional:

No template found for ListingsController#update, rendering head :no_content

Usually one redirects or renders something after a successful operation, that's what the normal "update" operation does: if you change something in the form and click "update listing" you'll end up in the index page, see

def process_after_update
# The following sets variables used in the index view. We render the
# index view directly to avoid having to redirect.
#
# Redirects lead to a race condition where we redirect to a cached view
# after updating data and we don't bust the cache fast enough before
# hitting the view, therefore stale content ends up being served from
# cache.
#
# https://github.com/forem/forem/issues/10338#issuecomment-693401481
@listings = listings_for_index_view
@listings_json = @listings.to_json(INDEX_JSON_OPTIONS)
render :index
end

Unfortunately though, the update action, exits early if the request from the frontend is bump (usually this should be its own separate Rails action with its own behavior):

return bump_listing(cost) if listing_params[:action] == "bump"

bump_listing() as you can imagine, does not render anything, hence that weird "204 No Content" side effect

def bump_listing(cost)
org = Organization.find_by(id: @listing.organization_id)
if org&.enough_credits?(cost)
charge_credits_before_bump(org, cost)
elsif current_user.enough_credits?(cost)
charge_credits_before_bump(current_user, cost)
else
process_no_credit_left && return
end
end

I think we should not make bump a special case and have the same exact behavior a regular update has, not sure what should change in the frontend to make that happen

@nickytonline nickytonline removed their assignment Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug always open for contribution internal team only internal tasks only for Forem team members size: estimated XS less than a few days to complete tech: frontend changes will primarily involve frontend technologies
Projects
No open projects
Current Cycle Work
In Progress 🖥
Development

No branches or pull requests

5 participants