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

1361 Add image deletion #1603

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

1361 Add image deletion #1603

wants to merge 8 commits into from

Conversation

tomaszsmy
Copy link
Contributor

No description provided.

@tomaszsmy tomaszsmy requested a review from a team as a code owner August 1, 2022 09:47
@tomaszsmy tomaszsmy requested review from jo-elimu, vrudas and sshivaditya2019 and removed request for a team August 1, 2022 09:47
@tomaszsmy tomaszsmy mentioned this pull request Aug 1, 2022
@codecov
Copy link

codecov bot commented Aug 1, 2022

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Comparison is base (816478d) 13.78% compared to head (f39c40f) 14.83%.

Files Patch % Lines
...u/web/content/multimedia/image/ImageComponent.java 81.48% 3 Missing and 2 partials ⚠️
.../content/multimedia/image/ImageEditController.java 0.00% 2 Missing ⚠️
...ontent/multimedia/image/ImageDeleteController.java 90.90% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1603      +/-   ##
============================================
+ Coverage     13.78%   14.83%   +1.05%     
- Complexity      454      482      +28     
============================================
  Files           257      258       +1     
  Lines          8503     8514      +11     
  Branches        881      882       +1     
============================================
+ Hits           1172     1263      +91     
+ Misses         7279     7196      -83     
- Partials         52       55       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nya-elimu nya-elimu self-requested a review August 10, 2022 11:30
Copy link
Member

@nya-elimu nya-elimu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting this when trying to delete an image that is being used by a storybook chapter:

13:58:27.789 [qtp1974701110-20] WARN  org.hibernate.engine.jdbc.spi.SqlExceptionHelper - SQL Error: -8, SQLState: 23504
13:58:27.789 [qtp1974701110-20] ERROR org.hibernate.engine.jdbc.spi.SqlExceptionHelper - integrity constraint violation: foreign key no action; FKCNLKSDIKTH82EC6NIA8TB3CPS table: STORYBOOKCHAPTER

@tomaszsmy Note that there are two ways an Image can be used in a storybooks: 1) as a cover image, or 2) as a chapter image.

Screen Shot 2022-08-11 at 1 57 04 PM

So we need to make it impossible to delete an Image if it is already being used in a storybook. Only images that are not used anywhere should be possible to delete.

Also, to make your task a bit easier, here you can see the various Image_ tables in the database:

> SHOW TABLES;
+--------------------------------------------+
| Tables_in_webapp-eng                       |
+--------------------------------------------+
| Application                                |
| ApplicationVersion                         |
| Application_literacySkills                 |
| Application_numeracySkills                 |
| Audio                                      |
| AudioContributionEvent                     |
| AudioPeerReviewEvent                       |
| Audio_Letter                               |
| Audio_Number                               |
| Audio_Word                                 |
| Audio_literacySkills                       |
| Audio_numeracySkills                       |
| Contributor                                |
| Contributor_roles                          |
| DbMigration                                |
| Device                                     |
| Emoji                                      |
| Emoji_Word                                 |
| Image                                      |
| ImageContributionEvent                     |
| Image_Letter                               |
| Image_Number                               |
| Image_Word                                 |
| Image_literacySkills                       |
| Image_numeracySkills                       |
| Letter                                     |
| LetterContributionEvent                    |
| LetterLearningEvent                        |
| LetterSoundCorrespondence                  |
| LetterSoundCorrespondenceContributionEvent |
| LetterSoundCorrespondencePeerReviewEvent   |
| LetterSoundCorrespondence_Letter           |
| LetterSoundCorrespondence_Sound            |
| Number                                     |
| NumberContributionEvent                    |
| NumberPeerReviewEvent                      |
| Number_Word                                |
| Sound                                      |
| SoundContributionEvent                     |
| StoryBook                                  |
| StoryBookChapter                           |
| StoryBookContributionEvent                 |
| StoryBookLearningEvent                     |
| StoryBookParagraph                         |
| StoryBookParagraph_Word                    |
| StoryBookPeerReviewEvent                   |
| Syllable                                   |
| Syllable_Sound                             |
| Video                                      |
| Video_Letter                               |
| Video_Number                               |
| Video_Word                                 |
| Video_literacySkills                       |
| Video_numeracySkills                       |
| Word                                       |
| WordContributionEvent                      |
| WordLearningEvent                          |
| WordPeerReviewEvent                        |
| Word_LetterSoundCorrespondence             |
+--------------------------------------------+
59 rows in set (0.00 sec)

These can also be seen in https://github.com/elimu-ai/webapp/blob/main/src/main/resources/META-INF/jpa-schema-export.sql

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

Successfully merging this pull request may close these issues.

2 participants