search

benjaffe / chrome-okc-plugin

OkCupid Poly Plugin
MIT License
63 stars 32 forks source link

Sanitize shockwave's pull request to add Predator Alert questions as a new category 'predator' #114

Closed k0pernikus closed 9 years ago

k0pernikus commented 9 years ago

I wanted to make the requested changes to the stalling pull request #109 of @shokwave to add the predator category.

It was said there that change to the manifest.json should suffice, yet reality was a bit different ;) I had to do a bit more to get the predator category to show and I would like some feedback on my current progress.

There is one issue I am still getting as of yet, which I cannot figure out: Often at the first page load of a profile, the number of the predator count is shown as NaN. On reloads it will be calculated correctly, and I have seen the number add up.

BTW: I apologize for refactoring so much in the process, it did made debugging easier. Please let me know if you would want to include those changes as well, or if I only shall add necessary changes as a separate pull request.

benjaffe commented 9 years ago

@k0pernikus This is wonderful! I've had no time to work on this over the last several months, and I'm super happy to see this PR. No apologies needed -- it was necessary refactoring.

I'll merge in this PR, and hopefully ship it soon. I'm not sure when I'll have time to track down the NaN issue, but I'll try to set aside a bit of time in the coming weeks to debug it. Obviously LMK if you figure it out :)

benjaffe commented 9 years ago

I'm not seeing the NaN issue yet. Are there special steps for reproducing the issue?

k0pernikus commented 9 years ago

I have it on first load of a profile. Reloads of the same profile most likely will work. I have two guesses, but am not sure:

k0pernikus commented 9 years ago

On first profile page load it looks like this:

Example of NaN issue

I just crosschecked and I noticed that indeed the answer for a question can show up in German, as in this case there will be "Nein" instead of "No".

Screenshot of PREDATOR tab with German answer

benjaffe commented 9 years ago

The extension does matching based on the question and answer text, so yes, translations are a problem for the plugin in general. There's an issue open about translations. I don't currently have time to refactor to properly handle translations.

I don't see how it'd relate to the NaN issue though. If the question doesn't match, it'll just skip the question and not affect the matching in any category.

Does it always happen on the first load of a profile? Or is it intermittent?

benjaffe commented 9 years ago

OH! Ha!! That's it!!

benjaffe commented 9 years ago

So ideally, if it finds a question match, but no answer match, it should skip. That's probably the refactoring that's needed.

I have to go to work, but I'm glad we figured that part out. Now it won't bug me all day. :)

instantgreen commented 9 years ago

@k0pernikus Chrome isn't my default browser so I just changed it's language to English and now I'm getting the English version of the site and the plugin works again.