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.

Sign up for GitHub

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

Jump to bottom

[BUG]: Problems with image when translating lessons into Portugese #19110

Open
chris7716 opened this issue Nov 4, 2023 · 14 comments · Fixed by #20188
Open

[BUG]: Problems with image when translating lessons into Portugese #19110

chris7716 opened this issue Nov 4, 2023 · 14 comments · Fixed by #20188
Assignees
@U8NWXD
Labels
bug Label to indicate an issue is a regression Impact: High Blocks or significantly slows down a core workflow. target: Q4 2024 Plan to complete by 24 Dec 2024. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet.

Comments

@chris7716
Copy link
Contributor

chris7716 commented Nov 4, 2023 •
edited by StephenYu2018
Loading

Describe the bug

When reviewing some of the cards, reviewers can not see images.

URL of the page where the issue is observed.

/contributor-dashboard

Steps To Reproduce

  1. Log in with the admin
  2. Go to the admin page under the "Roles" tab, and give yourself the following roles:
    • Curriculum Admin
    • Translation Admin
  3. Go to url /contributor-dashboard-admin and give yourself review translation rights in the Portugese language.
  4. Go to creator dashboard. Create and publish an exploration that contains a content card with an image with accessibility text.
  5. Go to topic and skills dashboard. Create a topic. Then, create a story in that topic. Create a chapter for that story and link the exploration to the chapter. Save story changes and finally publish the story.
  6. Log in with a different account.
  7. Go to the contributor dashboard page under the "Translate Text" tab, and submit a suggest in Portugese for the listed opportunity.
  8. Log in with the original admin account.
  9. Go to the contributor dashboard page under the "Review Translations" tab, and click on the opportunity listed. Finally, click on the listed suggestion. Observe that the image won't display and is replaced with the accessibility text.

Expected Behavior

I expect images to load and display properly within the translation suggestion review modal.

Screenshots/Videos

Captura de tela 2023-10-26 122857

What device are you using?

Desktop

Operating System

No response

What browsers are you seeing the problem on?

No response

Browser version

No response

Additional context

Debugging doc created by @StephenYu2018: https://docs.google.com/document/d/1sL74bmX1A7YLGrbLqY0yAuByZk3nCeIJl65V8sMmINI/edit?usp=sharing

Tips for developers

Before tackling the bug, please use git bisect (see https://git-scm.com/docs/git-bisect) to investigate which PR caused it (you only need to go back as far as commit https://github.com/oppia/oppia/commits/9a334e9). If you find the PR, leave a comment on this issue pointing to it, or just say that it happened before commit 9a334e9 if you could reproduce it there. This will help us fix the issue by reverting the faulty PR.

Also, if this is your first issue, please make sure to follow https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#choosing-a-good-first-issue and https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#setting-things-up before claiming it. Thanks!
@chris7716 chris7716 added triage needed bug Label to indicate an issue is a regression labels Nov 4, 2023
@abansal15
Copy link

hello @seanlip could you please assign this issue to me.

To solve the issue of images not displaying during the translation process in Portuguese lessons:

1.) Check image file paths and confirm their existence.
2.) Ensure that the image tags or code references are correctly linked to the images in the codebase.

@V0YD23
Copy link
Contributor

V0YD23 commented Dec 6, 2023

@seanlip can i work on this issue ?

@seanlip
Copy link
Member

seanlip commented Dec 6, 2023

@abansal15 @yash1378 Per the guidance at https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#choosing-a-good-first-issue, please provide an explanation of what your PR will do (with names of files you're changing, what you plan to change in each file, etc.). If it looks good, we can assign you to this issue.

Please also follow the other instructions on that wiki page if you have not yet done so. Thanks!

@V0YD23
Copy link
Contributor

V0YD23 commented Dec 6, 2023

sure sir i will go through this guide once and will not disturb you unnecessarily from now onwards

@seanlip seanlip added Impact: High Blocks or significantly slows down a core workflow. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. labels Dec 23, 2023
@pritam2317
Copy link
Contributor

Hello @chris7716 could please add steps to reproduce in the PR description, this issue seems interesting to me and i wanted to work.
Thank you

@StephenYu2018
Copy link
Member

@pritam2317 Here are the steps to reproduce the issue on your local server:

  1. Log in with the admin account.
  2. Go to the admin page under the "Roles" tab, and give yourself the following roles:
  • Curriculum Admin
  • Translation Admin
  1. Go to url /contributor-dashboard-admin and give yourself review translation rights in the Portugese language.
  2. Go to creator dashboard. Create and publish an exploration that contains a content card with an image with accessibility text.
  3. Go to topic and skills dashboard. Create a topic. Then, create a story in that topic. Create a chapter for that story and link the exploration to the chapter. Save story changes and finally publish the story.
  4. Log in with a different account.
  5. Go to the contributor dashboard page under the "Translate Text" tab, and submit a suggest in Portugese for the listed opportunity.
  6. Log in with the original admin account.
  7. Go to the contributor dashboard page under the "Review Translations" tab, and click on the opportunity listed. Finally, click on the listed suggestion. Observe that the image won't display and is replaced with the accessibility text.

@chris7716 Please add these steps to the issue description.

@StephenYu2018
Copy link
Member

@pritam2317 @yash1378 @abansal15 I started a debugging doc for this issue here. If you have discovered anything not mentioned in this document or in the issue thread, feel free to let me know!

@StephenYu2018 StephenYu2018 self-assigned this Jan 30, 2024
@StephenYu2018
Copy link
Member

@seanlip In regards to finding the earliest commit where this issue is reproducible, I have video proof that the issue existed as early as Dec 2022 when the Python version was last updated. The video shows where I got that commit from, what commit I'm checked out on during the manual test, and the occurrence of this issue.

image-not-loading-py-last-updated.mp4

@Ash-2k3
Copy link
Member

Ash-2k3 commented Mar 12, 2024

Hey @StephenYu2018, I have left a comment in debugging doc, PTAL, Thanks!

@seanlip seanlip moved this from Todo to In Progress in Contributor Experience Team Mar 19, 2024
@StephenYu2018 StephenYu2018 removed their assignment Apr 4, 2024
@StephenYu2018
Copy link
Member

I have unassigned myself as I have finished investigating this issue for now. My entire debugging doc currently spans 30+ pages, so for time's sake, I recommend reading just the "Solution" section of my debugging doc to anyone who wants to implement my solution for this issue. If you have any comments, questions or concerns about the issue or my solution, please @ me in a comment under this thread and reassign me to this issue.

@StephenYu2018 StephenYu2018 self-assigned this Apr 9, 2024
@StephenYu2018
Copy link
Member

I re-assigned myself as we've decided in our team meeting that I'll implement the solution that I came up with.

@StephenYu2018 StephenYu2018 mentioned this issue Apr 20, 2024
Merged
5 tasks
github-merge-queue bot pushed a commit that referenced this issue May 1, 2024
@StephenYu2018
Fix #19110: Copy translation's target exploration images regardless i…
bcb13ff 
…f submitter uploaded new images or not (#20188)

* Move backend code out of if statement so that images are copied regardless if there are new images introduced in the translation suggestion or not.

* Add check for GCS in Production to raise an exception when trying to copy from a source asset that does not exist.

* Re-raise exception from GCS COPY operation when source asset does not exist.

* Remove TODO comment for saving images before the suggestion is submitted.

* Clarify image protocol when submitting translations.

* Fix raised exception message.

* Replace TODO comment by adding new backend test.
@github-project-automation github-project-automation bot moved this from In Progress to Done in Contributor Experience Team May 1, 2024
@StephenYu2018
Copy link
Member

I'm going through with writing a beam job to fix the currently saved images not loading (newly submitted translations with copied images should not have this issue at the moment).

@StephenYu2018 StephenYu2018 reopened this Jun 11, 2024
@github-project-automation github-project-automation bot moved this from Done to In Progress in Contributor Experience Team Jun 11, 2024
@StephenYu2018 StephenYu2018 mentioned this issue Sep 4, 2024
Open
@Kanupriyaoppia Kanupriyaoppia mentioned this issue Sep 4, 2024
Closed
@StephenYu2018
Copy link
Member

I'm dropping this issue. The current problem is that existing images in our Google Cloud Storage (GCS) don't display despite the changes from my merged PR. We need a beam job to copy existing images from GCS /exploration/<target id>/assets/images/<filename> to GCS /exploration_suggestions/<target id>/assets/images/<filename> for each translation suggestion.

Beam Job Algorithm

For each translation suggestion:

  1. Get text content from suggestion
  2. Get all the filenames of the image tags from text content
  3. Get target id from suggestion
  4. For each filename from filenames:
  • If GCS /exploration_suggestions/<target id>/assets/images/<filename> does not exist and /exploration/<target id>/assets/images/<filename> does exist:
    • Copy GCS /exploration/<target id>/assets/images/<filename> to GCS /exploration_suggestions/<target id>/assets/images/<filename>

Displayed Output once the beam job finishes running:

  • For each (target) exploration:
    • exploration id
    • original exploration content
    • For each image that was copied:
      • image name: GCS src path --> GCS dest path

TODO:

  • Plan how to parse translation_html for image filenames (maybe the functionality should belong in BaseSuggestion?)
  • Implement gcs_io.CopyFile, gcs_io.CheckFilepath
  • Implement the beam job in its own file

@StephenYu2018 StephenYu2018 removed their assignment Oct 6, 2024
@StephenYu2018
Copy link
Member

Also, here are the files I changed as progress towards the beam job:

https://gist.github.com/StephenYu2018/4920afee258dbe55a3d6725e0e387a6d

@StephenYu2018 StephenYu2018 moved this from In Progress to Todo in Contributor Experience Team Oct 15, 2024
@seanlip seanlip added the target: Q4 2024 Plan to complete by 24 Dec 2024. label Oct 29, 2024
@seanlip seanlip moved this from Todo to In Progress in Contributor Experience Team Oct 29, 2024
@U8NWXD U8NWXD mentioned this issue Dec 4, 2024
Closed
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@U8NWXD U8NWXD

Labels
bug Label to indicate an issue is a regression Impact: High Blocks or significantly slows down a core workflow. target: Q4 2024 Plan to complete by 24 Dec 2024. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet.
Projects
Contributor Experience Team
Status: In Progress
Milestone
No milestone
8 participants
@seanlip @U8NWXD @chris7716 @pritam2317 @Ash-2k3 @StephenYu2018 @abansal15 @V0YD23