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

Link to reported media URL in API Admin is incorrect #2689

Closed
AetherUnbound opened this issue Jul 20, 2023 · 2 comments · Fixed by #3804
Closed

Link to reported media URL in API Admin is incorrect #2689

AetherUnbound opened this issue Jul 20, 2023 · 2 comments · Fixed by #3804
Assignees
Labels
💻 aspect: code Concerns the software code in the repository 🛠 goal: fix Bug fix 🟨 priority: medium Not blocking but should be addressed soon 🧱 stack: api Related to the Django API 🔒 staff only Restricted to staff members

Comments

@AetherUnbound
Copy link
Collaborator

AetherUnbound commented Jul 20, 2023

Description

It appears that many of the media URLs in our image/audio report tables display in the admin UI with the wrong URL. The URLs look something like https://openverse.org/v1/audio/<id> or https://openverse.org/v1/images/<id> for audio and images respectively. The paths themselves look like paths to the API, but the TLD directs to the frontend where every link 404s.

Reproduction

  1. Visit https://api.openverse.engineering/admin/api/imagereport/
  2. Observe the incorrect URLs

I was not able to reproduce this locally because reports made via the API show up with the correct domain.

Resolution

We'll need two separate fixes as part of this issue:

  1. Fix the issue that's causing the incorrect URLs to be inserted
  2. Correct the data which is currently wrong
@AetherUnbound AetherUnbound added 💻 aspect: code Concerns the software code in the repository 🔒 staff only Restricted to staff members 🛠 goal: fix Bug fix 🟧 priority: high Stalls work on the project or its dependents 🧱 stack: api Related to the Django API labels Jul 20, 2023
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Openverse Backlog Jul 20, 2023
@dhruvkb dhruvkb added 🟨 priority: medium Not blocking but should be addressed soon and removed 🟧 priority: high Stalls work on the project or its dependents labels Sep 26, 2023
@AetherUnbound AetherUnbound self-assigned this Feb 12, 2024
@AetherUnbound AetherUnbound moved this from 📋 Backlog to 📅 To Do in Openverse Backlog Feb 12, 2024
@AetherUnbound
Copy link
Collaborator Author

Ah, it looks like it might be coming from here:

url = (
f"{AbstractMediaReport.BASE_URL}v1/{media_type}/{self.media_obj.identifier}"
)

BASE_URL = config("BASE_URL", default="https://openverse.org/")

I'm going to see if we set this in production at all or if it's left as the default (which is what's causing the problem I think).

@AetherUnbound
Copy link
Collaborator Author

AetherUnbound commented Feb 16, 2024

Yep! That was it! I changed BASE_URL=https://openverse.org/ locally and submitted a report, and it's showing with the wrong URL. I'll get a PR here to change the default, and a PR in the infrastructure repo in case we're overwriting it there. Then we'll need to do a deployment and run either an SQL update or a data migration command to update the old data.

Update: Since this is actually a dynamic field and not one stored in the database, we can just update the configuration value and all the records should be updated! Yay!

@openverse-bot openverse-bot moved this from 📅 To Do to 🏗 In Progress in Openverse Backlog Feb 17, 2024
@openverse-bot openverse-bot moved this from 🏗 In Progress to ✅ Done in Openverse Backlog Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💻 aspect: code Concerns the software code in the repository 🛠 goal: fix Bug fix 🟨 priority: medium Not blocking but should be addressed soon 🧱 stack: api Related to the Django API 🔒 staff only Restricted to staff members
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants