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

Add placeholder image that is displayed when direct link is used for image that does not exist #290

Merged
merged 4 commits into from
Aug 8, 2021

Conversation

Coteh
Copy link
Owner

@Coteh Coteh commented Aug 1, 2021

I started working on this for #13, to provide a placeholder image thumbnail in comments section in instances where user commented on deleted image. Then I split this off due to scope, as well as the fact that this would be a good feature to have in not just comments section, but also for direct link accesses too. It seems like imgur takes this approach as well, and that's what I based my implementation off of.

Other Changes:

  • Created integration test utils, which contains just addImagesForUser right now. Modified User Images integ test to use this util function as well and ensured those tests still pass.
  • Added 500 status code error response if DB encountered error while finding image via direct link, an error response would be returned in place of the placeholder image in this case

…image that does not exist

I started working on this for #13, to provide a placeholder image thumbnail in comments section in instances where user commented on deleted image. Then I split this off due to scope, as well as the fact that this would be a good feature to have in not just comments section, but also for direct link accesses too. It seems like imgur takes this approach as well, and that's what I based my implementation off of.

Other Changes:
- Created integration test utils, which contains just `addImagesForUser` right now. Modified User Images integ test to use this util function as well and ensured those tests still pass.
- Added 500 status code error response if DB encountered error while finding image via direct link, an error response would be returned in place of the placeholder image in this case
- Also fix mistake in route.js where I limited the scope for image does not exist variable after adding try-catch scope (I added the try-catch scope so it reports a better error message if that particular image couldn't be loaded)
@codecov
Copy link

codecov bot commented Aug 1, 2021

Codecov Report

Merging #290 (f7d7206) into development (59ee025) will increase coverage by 2.08%.
The diff coverage is 93.75%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development     #290      +/-   ##
===============================================
+ Coverage        53.91%   56.00%   +2.08%     
===============================================
  Files               15       15              
  Lines             1137     1150      +13     
===============================================
+ Hits               613      644      +31     
+ Misses             524      506      -18     
Impacted Files Coverage Δ
lib/routes/route.js 45.27% <93.75%> (+3.24%) ⬆️
lib/database-ops.js 56.08% <0.00%> (+2.21%) ⬆️
lib/util.js 97.67% <0.00%> (+2.32%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59ee025...f7d7206. Read the comment docs.

test/integ/image_integ_test.js Show resolved Hide resolved
test/integ/image_integ_test.js Show resolved Hide resolved
test/integ/integ_test_utils.js Outdated Show resolved Hide resolved
test/integ/integ_test_utils.js Outdated Show resolved Hide resolved
test/integ/integ_test_utils.js Outdated Show resolved Hide resolved
test/integ/image_integ_test.js Show resolved Hide resolved
test/integ/image_integ_test.js Show resolved Hide resolved
@Coteh Coteh merged commit 7e30d29 into development Aug 8, 2021
@Coteh Coteh deleted the placeholder-image branch August 8, 2021 03:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant