Skip to content
This repository has been archived by the owner on Oct 29, 2020. It is now read-only.

Strip emojis from reportbacks #7014

Open
ngjo opened this issue Sep 7, 2016 · 19 comments
Open

Strip emojis from reportbacks #7014

ngjo opened this issue Sep 7, 2016 · 19 comments

Comments

@ngjo
Copy link
Contributor

ngjo commented Sep 7, 2016

FEATURE OVERVIEW

User Story

As a member, I should know that I can't submit an emoji in the RB caption or Why cuz it'll 404.

Additional Information (optional)

If you submit an emoji it will break the campaign page -- we're getting zendesk tickets from people who are saying after they try to submit a RB, the page 404s and we think it could be someone submitting emojis.

Easier (and interim) fix to #6167 !

Why This Matters

So the site doesn't break and the user knows that they can't put in an emoji.

Here's the text to add in the two places:

screen shot 2016-09-06 at 5 23 47 pm

screen shot 2016-09-06 at 5 23 56 pm

cc @hbghidey

@angaither
Copy link
Contributor

we might want to hold off on this, and wait till we are sending rbs to rogue.

@ngjo
Copy link
Contributor Author

ngjo commented Sep 7, 2016

@angaither with the hope that rogue will accept the emojis? if so, would that mean that ppl could send in emojis?!?!

cc @hbghidey

@angaither
Copy link
Contributor

@ngjo not the hope that rogue will accept emojis, the DEMAND that it does ;)

@ngjo
Copy link
Contributor Author

ngjo commented Sep 7, 2016

😂😂😂😂😂😂😂😂😂😂😂

i would say there must be a good gif for this, but im too busy lol'ing about that. YESSSSSS!!!! DEMANDS!!!!

Thanks @angaither !

@ngjo
Copy link
Contributor Author

ngjo commented Sep 30, 2016

@angaither know that Katie just worked on this for Rogue, but since it won't be turned on for at least some time, would it be possible to add this text in for the interim?

We've been getting a lot more RBs as everyone ramps up efforts to get ppl reportingback more, buttttt that has meant more people getting the 404 experience.

Let me know!

@angaither
Copy link
Contributor

@ngjo sure we can put this into the next sprint

@ngjo
Copy link
Contributor Author

ngjo commented Sep 30, 2016

woohoo @angaither thanks!

@DFurnes
Copy link
Contributor

DFurnes commented Oct 3, 2016

@ngjo @angaither Adding in a "don't use emoji" help text seems a bit hostile... would it make more sense to strip out the emoji in a form alter hook so they don't cause the database error?

iconv('UTF-8', 'ASCII//IGNORE//TRANSLIT', 'emoji 😅');  // "emoji "

@ngjo
Copy link
Contributor Author

ngjo commented Oct 3, 2016

@DFurnes hahaha, I guess that is a little aggressive! If we go the route you suggested, would the RB show up to be reviewable by the admin?

@DFurnes
Copy link
Contributor

DFurnes commented Oct 3, 2016

That will strip the emojis (and any other non-ASCII characters) from the string, so if we added that as a form alter hook Drupal would never attempt to save them to the database and cause things to blow up. That should mean the reportback would then be reviewable by the admin.

Sidnote: Ideally the //TRANSLIT option would use fallbacks for characters with passable ASCII equivalents (like é to e) but that doesn't seem to be reliably working when I tested it now. Switching the option order to ASCII//TRANSLIT//IGNORE does translate, but then it inserts ? for non-translatable chars. 🤔

@ngjo
Copy link
Contributor Author

ngjo commented Oct 3, 2016

Cool -- thanks for the clarification @DFurnes ! I'll let @angaither decide on this one as I know that Rogue will be able to accept emojis...so whatever's teh easiest! If we got hte route of additional text, I can sync w/ Ben to maybe have something less aggressive sounding.

@DFurnes
Copy link
Contributor

DFurnes commented Oct 3, 2016

Hah sorry, "aggressive" was probably aggressive terminology on my part! 😉 I just feel like we should try to work around any backend quirks if we can, rather than foisting that onto users. Definitely no need to rephrase if we decide to continue with this solution.

@angaither
Copy link
Contributor

@DFurnes yeah the ultimate goal here is to just allow reportbacks to have emoji in rogue
and this is a silly temp solution till rogue is accepting reportbacks.

@jessleenyc jessleenyc changed the title Additional text about how we can't accept emojis Strip emojis from reportbacks Oct 5, 2016
@itsjoekent
Copy link
Contributor

Had some problems getting this running, looking at the notes from the community iconv has quite a few bugs http://php.net/manual/en/function.iconv.php#usernotes

I'm gonna try use to some regex functions I found on SO instead, temp solution right? 😅

@ngjo
Copy link
Contributor Author

ngjo commented Oct 12, 2016

@aaronschachter
Copy link
Contributor

#7115 doesn't seem to be stripping emoji when posting to the /campaigns/:id/reportback that Gambit and the mobile apps use. We added emoji-strip to strip emoji's from Gambit submissions as a hotfix.

cc @mirie

@itsjoekent
Copy link
Contributor

@aaronschachter I'm guessing some new emoji;s have been added to the ecosystem since we merged this PR. Thanks for hot-fixing it on your end

@aaronschachter
Copy link
Contributor

FYI - Mobile Commons just released emoji support last week, so Gambit historically was never sending raw emoji until recently. Before the emoji release, Mobile Commons stripped them on their end when posting to Gambit.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants