Fix #19110: Copy translation's target exploration images regardless if submitter uploaded new images or not

[StephenYu2018]

Overview

This PR fixes #19110.

This PR does the following:

• Instructs SuggestionHandler to copy of translation's target exploration images within GCS file system from /exploration/... to /exploration_suggestions/...
  • This makes images created using the copy tool able to load when reviewing the translation suggestion
• Refactors relevant code flow starting from SuggestionHandler.post()
• cloud_storage_services.py and dev_mode_storage_services.py throw a ValueError if source asset does not exist at given filepath
• Introduces backend tests:
  • cloud_storage_services_test: Throws value error when source asset does not exist at filepath
  • suggestion_test:
  • test_saves_new_images_in_translation_that_were_copied_from_target_exploration
  • test_raises_exception_if_target_exploration_image_is_not_in_target_exploration_assets

The original bug occurred because images in the translation's target exploration were copied in GCS File System from /exploration/... to /exploration_suggestions/... only when the translation submitter uploaded new images to the translation text. If a translation reviewer were to review that submitted translation, the images would not load since they must be fetched from /exploration_suggestions/....

The PR that introduced this bug is #16532, in particular this commit. Prior to these changes, suggestion images were uploaded as long as it wasn't a question suggestion (in other words, a translation suggestion). Those changes introduced a check to only upload suggestion images if the list of new images introduced by the translation submitter was not empty. It's also possible that it was trying to check that the list of new images wasn't None.

Essential Checklist

Please follow the instructions for making a code change.

• [x] I have linked the issue that this PR fixes in the "Development" section of the sidebar.
• [x] I have checked the "Files Changed" tab and confirmed that the changes are what I want to make.
• [x] I have written tests for my code.
• [x] The PR title starts with "Fix #bugnum: " or "Fix part of #bugnum: ...", followed by a short, clear summary of the changes.
• [x] I have assigned the correct reviewers to this PR (or will leave a comment with the phrase "@{{reviewer_username}} PTAL" if I can't assign them directly).

Proof that changes are correct

Before:

After:

Proof of changes on desktop with slow/throttled network

https://github.com/oppia/oppia/assets/117241994/a0dc714f-a5fd-497a-8376-7706d4017061

Proof of changes on mobile phone

https://github.com/oppia/oppia/assets/117241994/bf81b67d-5f08-408f-a96d-9ea6640a5368

Proof of changes in Arabic language

https://github.com/oppia/oppia/assets/117241994/46076e6d-001e-4207-b6aa-cee2b4ca4df1

PR Pointers

• Never force push! If you do, your PR will be closed.
• To reply to reviewers, follow these instructions: https://github.com/oppia/oppia/wiki/Make-a-pull-request#step-5-address-review-comments-until-all-reviewers-approve
• Some e2e tests are flaky, and can fail for reasons unrelated to your PR. We are working on fixing this, but in the meantime, if you need to restart the tests, please check the "If your build fails" wiki page.
• See the Code Owner's wiki page for what code owners will expect.

[oppiabot[bot]]

Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this PR, it modifies files in jobs or platform folders.
Also @StephenYu2018, please make sure to fill in the server jobs form for the new job or feature to be tested on the backup server. This PR can be merged only after the test run is successful. Please refer to this guide for details.
Thanks!

[oppiabot[bot]]

Assigning @chris7716 for the first pass review of this PR. Thanks!

[StephenYu2018]

@chris7716 @kevintab95 If the only datastore change is to raise an error when copying files in the case of the given source asset not existing, do I still have to do server testing for this behavior? If so, then I think I'll just revert this behavior back to what it originally was, since I'm aiming to get this PR merged by April 30th latest.

[oppiabot[bot]]

Assigning @lkbhitesh07 for code owner reviews. Thanks!

[seanlip]

@chris7716 @kevintab95 If the only datastore change is to raise an error when copying files in the case of the given source asset not existing, do I still have to do server testing for this behavior? If so, then I think I'll just revert this behavior back to what it originally was, since I'm aiming to get this PR merged by April 30th latest.

Server testing isn't needed here -- thanks for checking.

[seanlip]

@chris7716 @lkbhitesh07 PTAL as codeowners, thanks.

[StephenYu2018]

@seanlip I've responded to some of your review comments asking for clarifications on a few things.

[oppiabot[bot]]

Unassigning @chris7716 since they have already approved the PR.

[oppiabot[bot]]

Hi @StephenYu2018, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to merge this PR once the CI checks pass and you're happy with it. Thanks!

[StephenYu2018]

@seanlip @chris7716 @kevintab95 Can you merge this PR now? The CI checks have passed.