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

Fix: Make favicon middleware backwards compatible with pngs #14736

Merged
merged 1 commit into from
Oct 31, 2022

Conversation

gu-stav
Copy link
Contributor

@gu-stav gu-stav commented Oct 28, 2022

What does it do?

Makes the switch to png favicons in #14655 backward compatible. In case the .png doesn't exist in the app it falls back to use the .ico version.

Why is it needed?

Ensures backward compatibility.

How to test it?

  1. Rename the favicon.png to favicon.ico from the getstarted example
  2. Restart the server
  3. Validate it does not crash or log an error.

Related issue(s)/PR(s)

@gu-stav gu-stav added source: core:admin Source is core/admin package pr: fix This PR is fixing a bug labels Oct 28, 2022
@gu-stav gu-stav added this to the 4.4.6 milestone Oct 28, 2022
@gu-stav gu-stav requested review from nathan-pichon, derrickmehaffy, petersg83, Convly and jhoward1994 and removed request for Convly October 28, 2022 09:38
@codecov
Copy link

codecov bot commented Oct 28, 2022

Codecov Report

Base: 58.79% // Head: 58.79% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (1eaea7a) compared to base (527b785).
Patch coverage: 32.00% of modified lines in pull request are covered.

❗ Current head 1eaea7a differs from pull request most recent head b6ee5a9. Consider uploading reports for the commit b6ee5a9 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #14736      +/-   ##
==========================================
- Coverage   58.79%   58.79%   -0.01%     
==========================================
  Files        1324     1327       +3     
  Lines       32091    32114      +23     
  Branches     5999     6007       +8     
==========================================
+ Hits        18869    18881      +12     
- Misses      11361    11371      +10     
- Partials     1861     1862       +1     
Flag Coverage Δ
front 62.46% <9.09%> (-0.02%) ⬇️
unit 50.38% <50.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...lder/admin/src/components/FormModal/forms/index.js 28.18% <0.00%> (ø)
packages/core/database/lib/metadata/index.js 17.54% <0.00%> (-2.46%) ⬇️
packages/core/utils/lib/import-default.js 100.00% <ø> (ø)
...s/FormModal/contentType/createContentTypeSchema.js 14.89% <11.11%> (-1.39%) ⬇️
...ges/core/utils/lib/__tests__/import-default/cjs.js 100.00% <100.00%> (ø)
...ges/core/utils/lib/__tests__/import-default/esm.js 100.00% <100.00%> (ø)
packages/core/utils/lib/index.js 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@derrickmehaffy derrickmehaffy left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@jhoward1994 jhoward1994 left a comment

Choose a reason for hiding this comment

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

Works well for me 👍

@gu-stav gu-stav merged commit 0aec9b3 into main Oct 31, 2022
@gu-stav gu-stav deleted the fix/favicon-png-backwards-compatible branch October 31, 2022 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: fix This PR is fixing a bug source: core:admin Source is core/admin package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error: ENOENT: no such file or directory, open '/workspace/favicon.png'
3 participants