Fix for instagram links #2849

albarnaz · 2018-10-08T21:03:09Z

Issue: #1760

Problem

There is no suggestion in the Instagram empty field in enterprise profile setup menu. Some users copy-paste the full URL on their page. That doesn't work because what is expected is only their Instagram user pseudo.

FIX

Add a suggestion/model in the empty field that guides the user so that he enter the Instagram info at the expected format (as it is done for Twitter for instance).

Add migration to fix the wrong links already inputted, from "http://instagram.com/myUser" to "@myUser"

luisramos0

welcome to OFN @albarnaz
Thanks for your PR!
EDIT: actually I should say Obrigado ;-)

mkllnk

Great work on the placeholders and the migration. I still think it would be good to sanitise the user input and extract the handle if the user pastes the URL. That can be done in a separate pull request though.

sauloperez · 2018-10-09T05:50:03Z

db/migrate/20181008201815_update_instagram_data.rb

+class UpdateInstagramData < ActiveRecord::Migration
+  def change
+    enterprises = Enterprise.where("instagram like ?", "%instagram.com%")
+    enterprises.each do |e|


We should rather use find_each here IMO. This can potentially be a large list of enterprises.

Ok, I agree with it. I will make this change.

sauloperez · 2018-10-09T05:51:07Z

Welcome aboard @albarnaz !

If you have any questions feel free to ask on the dev channel of our Slack team.

sigmundpetersen · 2018-10-18T10:20:16Z

Conflicts. Rebase needed? @albarnaz

daniellemoorhead · 2018-10-19T03:35:25Z

Can whoever stages this do a rebase beforehand perhaps? Not sure if you're still active on here @albarnaz?

albarnaz · 2018-10-19T12:56:02Z

@daniellemoorhead I already made a rebase, but I`m not understanding why it failed

sobrinho · 2018-10-19T13:02:16Z

git merge master
git rebase
git push --force

This should do the job

sobrinho · 2018-10-19T13:02:42Z

Or even git pull --rebase remote master where remote is openfoodfoundation remote (not your fork).

luisramos0 · 2018-10-30T17:42:51Z

staged https://staging.openfoodfrance.org

luisramos0 · 2018-11-07T09:40:40Z

what we agreed with @RachL and @Matt-Yorkley here is that we want to test the migration.
To do that we need to:
1 - before we stage to UK, have a look at staging UK data and make sure there are some entries with "broken data": instangram.com in the instagram field
2 - stage (which includes the data migration)
3 - verify on the app the instagram.com part is gone from the entries identified on the step 1

I'll can do this but probably next week only.

mkllnk · 2018-11-08T06:40:28Z

Staged on https://staging.openfoodnetwork.org.au/.

RachL · 2018-11-08T12:01:25Z

@mkllnk do you know which data were broken before? (see Luis' post above)

sstead · 2018-11-08T22:22:32Z

@maikel I've just had a look and from what I can see, now that the PR is staged it's correctly turning https://www.instagram.com/bravefoods/ into @bravefoods . However as Rachel said, should we check also that existing twitter links in the longhand format are getting fixed? There's very little data on staging, so I cant be sure that any of the profiles were previously in the longhand. Perhaps we create some profile with longhand links and then restage the PR? Or another idea?

mkllnk · 2018-11-08T23:31:43Z

Okay, Sally and I are trying out this new feature of the Australian staging server. When master code is on staging, the database is not reset. It's used as new baseline. We can use that here:

Stage master.
Create test data we need.
Stage pull request.
Observer effect of migrations / fixes etc.

These Instagram links are probably a good example of test data we don't need to specify in the code as developers. @sauloperez Would you agree?

sstead · 2018-11-09T02:12:26Z

Testing Notes
We have a problem...
In order for an instagram link to work the Social Settings must be in the format "happyfarm". Not @happyfarm or https://www.instagram.com/happyfarm or www.instagram.com/happyfarm or /happyfarm

#2859 has been merged, and now the form will convert any new http or www. instagram links into @happyfarm format. This format leads to badges on profiles that are broken. Also, after this PR, now if a profile has their instagram link in the correct format "happyfarm", they'll get an error message when they try to save anything in their Enterprise Settings.

I've tested this PR and it's doing what it intends to do - any pre-existing instagram links in the http or www format are converted into @happyfarm links. However, links must be in happyfarm format, not @happyfarm format to work.

So what do we need to do?

2859 should have converted all new links to the plain "happyfarm" format, not "@happyfarm". Can we redo this?
2859 should only give an error message if the Instagram field contains something that cannot be converted to the correct "happyfarm" format. In theory we should be able to convert any kind of link into the correct 'happyfarm' link- including http, www, @happyfarm, or /happyfarm
This PR should convert any pre-existing incorrect links (http, www. @happyfarm or /happyfarm) into the correct "happyfarm" format
This PR - The prompt in the form should be as below:

Testing notes (nothing not described above)
https://docs.google.com/document/d/1TkmhGczL8t78UnH2VvspQorI1ysp3BjUfsRelzYIETk/edit#

sstead · 2018-11-09T03:06:47Z

@mkllnk has reversed #2859 .
I have updated the issue description on #1760 so that it is more clear. Back to you @albarnaz

daniellemoorhead · 2018-12-14T05:06:45Z

Hey @albarnaz 👋
How are you doing with this? Any thoughts on how to move it forward? Have you got some time to finish the work so we can get it reviewed and tested and hopefully merged?

This time of year is a busy one, so totally understand if time got away from you. Let us know if you're able to proceed with this or if we should close it for now and let someone else pick up where you left off 😄

albarnaz · 2018-12-14T11:58:38Z

Hi @daniellemoorhead
I'm going to work on this, I'm sorry I stayed away for a while

mkllnk

The code looks good. This is ready for testing again.

I ran the migration against the Australian and French production data. It does improve the data, but there are more unhandled cases. This is the conversion on French data:

https://www.instagram.com/conciergerie_de_quartier/ => conciergerie_de_quartier
https://www.instagram.com/conciergerie_de_quartier/ => conciergerie_de_quartier
https://www.instagram.com/geoffrey_martin_74 => geoffrey_martin_74
https://www.instagram.com/clac_conserverie/ => clac_conserverie
https://www.instagram.com/paradisesuperfoods/ => paradisesuperfoods
https://www.instagram.com/ => www.instagram.com
https://www.instagram.com/paradisesuperfoods/ => paradisesuperfoods
https://www.instagram.com/la_panetiere_des_hameaux/ => la_panetiere_des_hameaux
https://www.instagram.com/la_panetiere_des_hameaux/ => la_panetiere_des_hameaux
https://instagram.com/https%3A%2F%2Fwww.instagram.com%2Forangesbio%2F%3Fhl%3Des?fbclid=IwAR35hFjCShjbsS9dL4E914o8OGoPwep_0_m0F0wmulmmHmE4PRlYr04KsZc => https%3A%2F%2Fwww.instagram.com%2Forangesbio%2F%3Fhl%3Des?fbclid=IwAR35hFjCShjbsS9dL4E914o8OGoPwep_0_m0F0wmulmmHmE4PRlYr04KsZc

We still need proper validation for this field.

The commit history contains a few detours now and should be squashed.

RachL · 2018-12-20T12:55:48Z

Deploying on UK staging

RachL · 2018-12-20T13:13:38Z

Alright, didn't forgot front office this time :) As I understand migration has been tested, right @mkllnk ? But indeed we don't have a validation on this field. But in the meantime, new user have now a correct placeholder, which should prevent errors for them.

https://docs.google.com/document/d/1ArkpTlOG94yKE8eucDs1RP476XKxBrg6EowF-KV2ubw/edit#

So I'm moving this to ready to go, and will open another issue for the validation part.

sauloperez · 2018-12-20T15:49:16Z

@mkllnk do you still want these commits to be squashed? Let's wait for him to ack before merging it.

albarnaz · 2018-12-20T19:20:09Z

@sauloperez I will squash the commits in just one.

mkllnk · 2018-12-21T03:41:17Z

Yes, we can squash it through Github as well, by selecting it on the merge button.

sigmundpetersen assigned albarnaz Oct 8, 2018

sobrinho approved these changes Oct 8, 2018

View reviewed changes

luisramos0 approved these changes Oct 8, 2018

View reviewed changes

JuarezLustosa approved these changes Oct 8, 2018

View reviewed changes

mkllnk approved these changes Oct 9, 2018

View reviewed changes

sauloperez suggested changes Oct 9, 2018

View reviewed changes

sauloperez approved these changes Oct 9, 2018

View reviewed changes

sigmundpetersen mentioned this pull request Oct 18, 2018

Instagram pattern #2859

Merged

albarnaz force-pushed the instagram_links branch from e1db89b to 94c2c41 Compare October 18, 2018 18:50

albarnaz force-pushed the instagram_links branch from 94c2c41 to f0a47be Compare October 19, 2018 13:18

luisramos0 added the pr-staged-fr staging.coopcircuits.fr label Oct 30, 2018

luisramos0 removed the pr-staged-fr staging.coopcircuits.fr label Nov 6, 2018

mkllnk added the pr-staged-au staging.openfoodnetwork.org.au label Nov 8, 2018

mkllnk mentioned this pull request Nov 9, 2018

Revert "Instagram pattern" #3007

Merged

sstead mentioned this pull request Nov 9, 2018

Instagram links not working #1760

Closed

mkllnk removed the pr-staged-au staging.openfoodnetwork.org.au label Nov 12, 2018

fabricio.albarnaz added 12 commits December 14, 2018 11:10

Add new attribute instagram_placeholder to local yml

ba380e4

Add placeholder to instagram link input

8238a1f

Add migration to fix wrong instagram links

cb2e8e0

Add new attribute facebook_placeholder to local yml

d0f0bbd

Add placeholder to facebook link input

ee9f96f

Add new attribute linkedin_placeholder to local yml

6df2018

Add placeholder to linkedin link input

c08704a

Change each for find_each in migration

c3b166f

Add new attribute instagram_placeholder to local yml

04d695f

Add migration to fix wrong instagram links

74f3e35

Add new attribute linkedin_placeholder to local yml

83e9905

Add placeholder to linkedin link input

36b564f

albarnaz force-pushed the instagram_links branch from 7470fb2 to 36b564f Compare December 14, 2018 13:19

sobrinho approved these changes Dec 14, 2018

View reviewed changes

mkllnk approved these changes Dec 18, 2018

View reviewed changes

RachL self-assigned this Dec 20, 2018

RachL added the pr-staged-uk staging.openfoodnetwork.org.uk label Dec 20, 2018

RachL removed the pr-staged-uk staging.openfoodnetwork.org.uk label Dec 20, 2018

sauloperez added the blocked label Dec 20, 2018

mkllnk removed the blocked label Dec 21, 2018

mkllnk merged commit 5d51571 into openfoodfoundation:master Dec 21, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix for instagram links #2849

Fix for instagram links #2849

albarnaz commented Oct 8, 2018

luisramos0 left a comment •

edited

Loading

mkllnk left a comment

sauloperez Oct 9, 2018

albarnaz Oct 9, 2018

sauloperez commented Oct 9, 2018

sigmundpetersen commented Oct 18, 2018

daniellemoorhead commented Oct 19, 2018

albarnaz commented Oct 19, 2018

sobrinho commented Oct 19, 2018

sobrinho commented Oct 19, 2018

luisramos0 commented Oct 30, 2018

luisramos0 commented Nov 7, 2018

mkllnk commented Nov 8, 2018

RachL commented Nov 8, 2018

sstead commented Nov 8, 2018

mkllnk commented Nov 8, 2018

sstead commented Nov 9, 2018 •

edited

Loading

sstead commented Nov 9, 2018

daniellemoorhead commented Dec 14, 2018

albarnaz commented Dec 14, 2018

mkllnk left a comment

RachL commented Dec 20, 2018

RachL commented Dec 20, 2018 •

edited

Loading

sauloperez commented Dec 20, 2018

albarnaz commented Dec 20, 2018

mkllnk commented Dec 21, 2018

Fix for instagram links #2849

Fix for instagram links #2849

Conversation

albarnaz commented Oct 8, 2018

Issue: #1760

Problem

FIX

luisramos0 left a comment • edited Loading

Choose a reason for hiding this comment

mkllnk left a comment

Choose a reason for hiding this comment

sauloperez Oct 9, 2018

Choose a reason for hiding this comment

albarnaz Oct 9, 2018

Choose a reason for hiding this comment

sauloperez commented Oct 9, 2018

sigmundpetersen commented Oct 18, 2018

daniellemoorhead commented Oct 19, 2018

albarnaz commented Oct 19, 2018

sobrinho commented Oct 19, 2018

sobrinho commented Oct 19, 2018

luisramos0 commented Oct 30, 2018

luisramos0 commented Nov 7, 2018

mkllnk commented Nov 8, 2018

RachL commented Nov 8, 2018

sstead commented Nov 8, 2018

mkllnk commented Nov 8, 2018

sstead commented Nov 9, 2018 • edited Loading

sstead commented Nov 9, 2018

daniellemoorhead commented Dec 14, 2018

albarnaz commented Dec 14, 2018

mkllnk left a comment

Choose a reason for hiding this comment

RachL commented Dec 20, 2018

RachL commented Dec 20, 2018 • edited Loading

sauloperez commented Dec 20, 2018

albarnaz commented Dec 20, 2018

mkllnk commented Dec 21, 2018

luisramos0 left a comment •

edited

Loading

sstead commented Nov 9, 2018 •

edited

Loading

RachL commented Dec 20, 2018 •

edited

Loading