fablabbcn / smartcitizen-api

The Smart Citizen Engine
https://developer.smartcitizen.me
GNU Affero General Public License v3.0
10 stars 4 forks source link

User profile image urls #166

Closed pral2a closed 9 months ago

pral2a commented 4 years ago

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

viktorsmari commented 4 years ago
  1. See pr #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.

    https://github.com/fablabbcn/smartcitizen-api/blob/88f9a16288d9c6a4963136c1bb80d9658afe3102/app/views/v0/users/_user.jbuilder#L16-L17

  4. This is a rails thing and might change.
pral2a commented 4 years ago
pral2a commented 3 years ago

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

oscgonfer commented 1 year ago

Related? https://github.com/fablabbcn/smartcitizen-api/issues/223

oscgonfer commented 1 year ago

Hi @timcowlishaw

After reviewing https://github.com/fablabbcn/smartcitizen-api/pull/239, I feel we are missing something still. Is there a migration to do for this to work? Currently:

imagen

timcowlishaw commented 1 year ago

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 oldavatarproperty (pre-activestorage) rather thanprofile_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:

viktorsmari commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 11 months ago

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:

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

Sentry error for the above: https://iaac.sentry.io/issues/4576320257/?project=4506116341432320&query=is%3Aunresolved&referrer=issue-stream&statsPeriod=14d&stream_index=1

timcowlishaw commented 11 months ago

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

timcowlishaw commented 11 months ago

Once https://github.com/fablabbcn/smartcitizen-web/pull/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 commented 9 months ago

Closing as @timcowlishaw added this to the #286