Charcoal-SE / metasmoke

Web dashboard for SmokeDetector.
https://metasmoke.erwaysoftware.com
Creative Commons Zero v1.0 Universal
43 stars 34 forks source link

No Method Error - Invalidated Feedback #441

Closed SulphurDioxide closed 6 years ago

SulphurDioxide commented 6 years ago

When visiting the invalidated feedback page with a ?page parameter, like the one added by the pagination buttons at the bottom, I get the following error message:

NoMethodError in Admin#recently_invalidated

This link should present the error in question. You should also be able to trigger this error when clicking on any of the pagination links at the bottom of the invalidated feedback page.

thesecretmaster commented 6 years ago

@Undo1 Possible that this is related to Yvette's deleted account? This is caused by the feedback still having a user_id, but that user_id not matching an actual user since Yvette is gone. ~A fix would be to run Feedback.where(user_id:10).update(user_id:nil, user_name:nil) (username removed for future anonymity)~ This should be done from MySQL because AR wants to do boatloads of updates. Or, on line 20 of app/views/admin/recently_invalidated.html.erb, it could be changed from f.user.name to f.user_name

Undo1 commented 6 years ago

@thesecretmaster Could we change it to f.user&.name to handle null users?

j-f1 commented 6 years ago

@Undo1 f.user_name could be present even if f.user is nil.

thesecretmaster commented 6 years ago

@j-f1 As it is, in this case. But I think we wanted to fully remove Yvette from our system (GRPR?), so maybe in the future feedbacks should be anonymized.

j-f1 commented 6 years ago

@thesecretmaster In that case, couldn’t we find all feedbacks with a specific user_name and null the username out?

thesecretmaster commented 6 years ago

Yeah. I don't know if there's an actual anonymization process that exists, but if there is, it shouldn't fully nuke the user, just remove email+password+refrences to name scattered around, like feedback user_name and leave the user record, to avoid breaking stuff like this.

ArtOfCode- commented 6 years ago

Anonymisation policy is "either you give us the info we need or you don't have an account"; see /privacy on metasmoke.

thesecretmaster commented 6 years ago

@ArtOfCode- But "you don't have an account... but we store your name" is an edge case which exists here. Username is still stored with feedback (why?)

ArtOfCode- commented 6 years ago

@thesecretmaster Because chat exists, and not all chat users have metasmoke accounts.

thesecretmaster commented 6 years ago

Ok. Well, anyways, things to do to solve the problem. @Undo1 Can you confirm that on the prod DB, there are feedbacks with user_name and a user_id that pointed to a non-existant user? If so, can you set user_id to null? That should make the view automagically omit that field, which I'll push a fix for.

Nulling the user_id should avoid problems elsewhere/in the future, but even if you don't, my changes should still fix the issue by checking if the user referenced exists.

thesecretmaster commented 6 years ago

Fix deployed. @Undo1 could still do some majik fixing in the database, but the issue is resolved.