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

User profile image urls #166

Closed
pral2a opened this issue Apr 30, 2020 · 18 comments
Closed

User profile image urls #166

pral2a opened this issue Apr 30, 2020 · 18 comments
Labels
Milestone

Comments

@pral2a
Copy link
Member

pral2a commented Apr 30, 2020

User profile JSON blob returned by /v0/me

{
    "avatar": "https://images.smartcitizen.me/s100/avatars/b4b/1djs79t.lab.png",
    "devices": [],
    "email": "..",
    "id": 148,
    "joined_at": "2013-06-06T19:59:29Z",
    "legacy_api_key": "...",
    "location": {},
    "profile_picture": "http://api.smartcitizen.me/rails/active_storage/blobs/eyJfcmFpbHMiOnsibWVzc2FnZSI6IkJBaHBPZz09IiwiZXhwIjpudWxsLCJwdXIiOiJibG9iX2lkIn19--d90982f085da9d5c2183c726d3245f51fa39cdc0/25a19eb09b28c28d77b99e5bda1b8d04_200x200.jpeg",
    "profile_picture2": "http://api.smartcitizen.me/rails/active_storage/representations/eyJfcmFpbHMiOnsibWVzc2FnZSI6IkJBaHBPZz09IiwiZXhwIjpudWxsLCJwdXIiOiJibG9iX2lkIn19--d90982f085da9d5c2183c726d3245f51fa39cdc0/eyJfcmFpbHMiOnsibWVzc2FnZSI6IkJBaDdCam9MY21WemFYcGxTU0lNTVRBd2VERXdNQVk2QmtWVSIsImV4cCI6bnVsbCwicHVyIjoidmFyaWF0aW9uIn19--74c34520cbe1b1037fa838d7b2a3692d8db5b794/25a19eb09b28c28d77b99e5bda1b8d04_200x200.jpeg",
    "role": "admin",
    "updated_at": "2019-09-27T13:12:01Z",
    "url": "http://pral2a.com",
    "username": "pral2a",
    "uuid": "b4bb2a6f-c3ef-4964-bc79-e8d78f0e66d2"
}
  1. User profile image urls in http causing the front-end to switch from http to https when loading the profile_picture or profile_picture2

Screenshot 2020-04-30 at 11 55 48

  1. User profile contains the avatar property with a deprecated domain / service "https://images.smartcitizen.me/s100/avatars/b4b/1djs79t.lab.png"

  2. Why we have profile_picture or profile_picture2?

  3. I suggest we clean this path or document it properly /rails/active_storage/representations/?

Topic 1 is critical, topics 2-4 are not urgent but will be useful to address them

@pral2a pral2a added the bug label Apr 30, 2020
viktorsmari added a commit that referenced this issue Apr 30, 2020
Was causing http urls instead of https. see:
#166

Solution is to force use SSL, and forward it from Nginx to the container
See more: rails/rails#22965
@viktorsmari
Copy link
Collaborator

viktorsmari commented Apr 30, 2020

  1. See pr Force SSL + Forward SSL from Nginx to the container #167
  2. avatar is the old attribute before we switched to use Active Storage.
    It is used as a fallback in the Angular app:
     <img ng-src="{{ vm.user.profile_picture || vm.user.avatar || './assets/images/avatar.svg' }}">
    Ideally we should copy all the pictures from the avatar which is a string, to the new one profile_picture which is an ActiveStorage object, but not sure if it's worth the effort. We can revisit.
  3. It was a test at the time, for using different size thumbnails if needed:
    It's not in use, so we can simply remove that one line, there is no other reference to it.

json.profile_picture request.base_url + url_for(user.profile_picture)
json.profile_picture2 request.base_url + url_for(user.profile_picture.variant(resize:"100x100"))

4. This is a rails thing and might change.

@pral2a
Copy link
Member Author

pral2a commented May 4, 2020

  • 1. OK
  • 2. I suggest you work on it to clean things up
  • 3. Yes, let's leave only one or at least document this properly on the api docs
  • 4. Let's leave it as it is and we can see how Rails 6 follows with it

@pral2a
Copy link
Member Author

pral2a commented Apr 19, 2021

upload.rb is deprecated. Consider removal on the upcoming refurbish

@oscgonfer oscgonfer mentioned this issue Mar 1, 2023
@oscgonfer oscgonfer added this to the 0723 milestone May 4, 2023
@oscgonfer
Copy link
Contributor

oscgonfer commented May 4, 2023

Related? #223

@oscgonfer
Copy link
Contributor

Hi @timcowlishaw

After reviewing #239, I feel we are missing something still. Is there a migration to do for this to work? Currently:

imagen

@timcowlishaw
Copy link
Contributor

Hey @oscgonfer - i've taken a look this morning, it appears to be unrelated to that PR, but definitely a related bug: `` profiles that used the old avatar property (pre-activestorage) rather than `profile_picture` don't show as the images.smartcitizen.me DNS record has gone away. Any idea where this used to point to?

See for example https://images.smartcitizen.me/s100/avatars/701/1citvht.10407640_420278614786473_8707648267347732669_n.jpg on the profile https://smartcitizen.me/users/6028.

We would need to work out where that pointed to (an AWS bucket, i'm guessing?) and if the images still exist we could either:

  • resurrect that domain as a 'legacy' image host, and optionally do something in the rails app to deprecate that 'avatar property and ensure that 'profile_picture' either points to activestorage or to the images.sc host transparently.

  • Write a script to import all the old images as activestorage blobs then shut down images.sc and remove the avatar field (probably preferable).

Any idea whereabouts the images might be, or where that DNS record originally pointed to?

@viktorsmari
Copy link
Collaborator

There is a smartcitizen-images app on Heroku that has not been changed since 2015

The app appears to be down:
https://smartcitizen-images.herokuapp.com/

Here is the app on Heroku:
https://dashboard.heroku.com/apps/smartcitizen-images

It might need some updates

image

Last line from the log file:

2023-07-25T08:50:58.840090+00:00 heroku[router]: at=error code=H14 desc="No web processes running" method=GET path="/favicon.ico" host=smartcitizen-images.herokuapp.com request_id=b87dcf47-f78d-4e0a-8716-879da28304ef fwd="46.22.107.156" dyno= connect= service= status=503 bytes= protocol=https

@timcowlishaw
Copy link
Contributor

Hi there @viktorsmari - aha thanks for that - any idea where the source code for that lives, and would you be able to give me access to the repo, and to the application on heroku? I'll investigate further. Many thanks!

Tim

@viktorsmari
Copy link
Collaborator

I did not find it here:
https://github.com/orgs/fablabbcn/repositories?language=&page=2&q=smartc&sort=&type=all

Maybe the code is hosted directly on Heroku, and not on Github?

Maybe @pral2a can give you access to the Heroku dashboard?

@pral2a
Copy link
Member Author

pral2a commented Aug 11, 2023

Hi @timcowlishaw @viktorsmari

That was a middleware written in Node to abstract S3

I though that was fully deprecated in favour of Rails Active Storage Overview but maybe some legacies remain

I sent a tarball of the Heroku source

Thank! :)

@timcowlishaw
Copy link
Contributor

timcowlishaw commented Oct 26, 2023

Finally looking at this, sorry folks for the delay.

We've got two related problems, as i see it:

  1. None of the avatars are available publicly at their URL any more (due to that heroku app going offline) and somewhere in the front-end, the avatar takes precedence over the profile_picture when both are set, so eg in @pral2a's case, although he has a profile picture, it doesn't show because the (broken) avatar is displayed.

  2. Several accounts (the majority of those that have any image set) have only the (deprecated, broken) avatar and no profile picture:

sc_dev=# select count(users.id), users.avatar_url is not null as has_avatar, active_storage_attachments.id is not null as has_profile_pic from users left join active_storage_attachments on active_storage_attachments.record_type='User' and active_storage_attachments.record_id=users.id group by has_avatar, has_profile_pic;
 count | has_avatar | has_profile_pic
-------+------------+-----------------
     6 | t          | t
  7463 | f          | f
   108 | f          | t
   357 | t          | f
(4 rows)

Therefore I think we need to tackle this in steps:

  • 1) Update the front-end to remove any references to avatar and use the profile_picture consistently
  • 2) Migrate all the old avatar images to the profile_picture field where no profile_picture already exists with a script to be run on prod which will grab the avatar from the old S3 bucket and add it with activestorage as a profile_picture (this can happen concurrently with (1) but the difference won't be seen 'til (1) is done.
  • 3) Remove all references to the avatar from the API codebase (once 1 and 2 are done).

I'm happy to start working on (2) and get ready for (3) when ready - would you like me to dip my toe into the angular app and see if i can handle (1) too, or is someone else better placed to do that? (cc @oscgonfer )

@timcowlishaw
Copy link
Contributor

Ok, i started working on (1) to see if i could find my way around the web-app and this has thrown up another interesting thing - it appears I'm getting hit by the rate limit when just browsing the site, and this is also causing images (and other things) not to load, which may also be causing problems!

@timcowlishaw
Copy link
Contributor

Yep, confirmed - i think this might be a more urgent thing to sort out - @oscgonfer we can chat this morning!

Screenshot 2023-10-26 at 08 31 02

@timcowlishaw
Copy link
Contributor

Another possible related thing - some users raise an error for having a profile_picture blob without a service_name property. Worth investigating at the same time. This happens on this search on staging (But not locally, for some resason): https://staging-api.smartcitizen.me/v0/users?q%5Busername_cont%5D=team

@timcowlishaw
Copy link
Contributor

Ok, (2) above is deployed - i'll do (1) and (3) early next week!

@timcowlishaw
Copy link
Contributor

Once fablabbcn/smartcitizen-web#448 and #277 are merged this should all work fine, and then i can get to work removing all trace of the old avatars from the api.

@oscgonfer
Copy link
Contributor

Closing as @timcowlishaw added this to the #286

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

No branches or pull requests

4 participants