Fix missing Publisher/Admin favicon and Admin copyright year (#4972)#1332
Open
Tharsanan1 wants to merge 3 commits intowso2:mainfrom
Open
Fix missing Publisher/Admin favicon and Admin copyright year (#4972)#1332Tharsanan1 wants to merge 3 commits intowso2:mainfrom
Tharsanan1 wants to merge 3 commits intowso2:mainfrom
Conversation
The Publisher and Admin entry templates emitted only the legacy <link rel="shortcut icon"> tag. Modern browsers prefer <link rel="icon" type="image/png"> and some contexts do not fall back, so the tab icon did not render. DevPortal already emits both tags; this change mirrors that pattern on Publisher and Admin. Fixes wso2/api-manager#4972 (bug 1) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Admin Copyright component's defaultMessage still said "© 2025" while Publisher and DevPortal were already rolled forward to 2026. Updated the JSX defaultMessage so the rebuilt admin bundle carries the correct year. Fixes wso2/api-manager#4972 (bug 2) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Un-skip and rewrite the dormant Publisher Footer test to assert the rendered text carries the expected year. - Add Issue4972/favicon.test.js: scans all six portal entry templates (Publisher/Admin/DevPortal x index.jsp.hbs + index.html/index.ejs) and asserts each emits both <link rel="shortcut icon"> and <link rel="icon" type="image/png"> referencing _favicon.png, and that the _favicon.png asset exists in each portal's site/public/images/. - Add Issue4972/copyrightYear.test.js: source-scan regression guard derives the expected year from Publisher's Footer.jsx and asserts Admin and DevPortal Copyright components carry the same year. - Add Issue4972/localeYear.test.js: locale-bundle regression guard scans every portal's locales/*.json for the product_details key. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tharsanan1
requested review from
Krishanx92,
lasanthaS,
piyumaldk and
tharikaGitHub
as code owners
📝 WalkthroughOverviewThis PR fixes two UI issues in the WSO2 API Manager Publisher and Admin portals:
ChangesHTML/Template Files
Copyright Year Update
Test Coverage
Verification
WalkthroughThis pull request adds favicon support enhancements and updates copyright year information across multiple portals. Changes include: adding PNG favicon declarations ( 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@portals/publisher/src/main/webapp/source/Tests/Unit/Issue4972/favicon.test.js`:
- Around line 82-91: The two tests ("emits the modern <link rel=\"icon\"
type=\"image/png\"> tag" and "points the modern <link rel=\"icon\"> at
_favicon.png") are too loose because they allow `_favicon.png` to appear
anywhere; tighten the second assertion to match a single <link ...> tag that
contains rel="icon", type="image/png", and the expectedHref together. Replace
the toContain(expectedHref) check on markup with a toMatch using a RegExp that
looks for a single <link[^>]* rel=["']icon["'][^>]* type=["']image\/png["'][^>]*
href=["']<expectedHref>["'][^>]*>, and if expectedHref may contain
regex-significant chars create/use an escapeRegExp utility to escape
expectedHref before interpolating into the RegExp.
In
`@portals/publisher/src/main/webapp/source/Tests/Unit/Issue4972/localeYear.test.js`:
- Around line 55-80: The test silently skips missing portal locale directories
in collectLocaleFiles (portals array in collectLocaleFiles) which can hide
missing admin/devportal coverage; update the test or collectLocaleFiles so it
asserts per-portal presence: for each portal in portals (publisher, admin,
devportal) check the directory at path.join(portalsRoot,
`${portal}/src/main/webapp/site/public/locales`) exists and contains at least
one .json, and fail the test if any portal directory is missing or empty
(reference collectLocaleFiles, portals array, and localeFiles variable to
implement this per-portal existence/coverage check).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d935594a-38cf-4183-b60a-2fbde5221fe0
⛔ Files ignored due to path filters (1)
portals/publisher/src/main/webapp/source/src/app/components/Base/Footer/__snapshots__/Footer.test.jsx.snapis excluded by!**/*.snap
📒 Files selected for processing (9)
portals/admin/src/main/webapp/admin/index.ejsportals/admin/src/main/webapp/site/public/pages/index.jsp.hbsportals/admin/src/main/webapp/source/src/app/components/Base/index.jsxportals/publisher/src/main/webapp/publisher/index.htmlportals/publisher/src/main/webapp/site/public/pages/index.jsp.hbsportals/publisher/src/main/webapp/source/Tests/Unit/Issue4972/copyrightYear.test.jsportals/publisher/src/main/webapp/source/Tests/Unit/Issue4972/favicon.test.jsportals/publisher/src/main/webapp/source/Tests/Unit/Issue4972/localeYear.test.jsportals/publisher/src/main/webapp/source/src/app/components/Base/Footer/Footer.test.jsx
Comment on lines
+82
to
+91
| it('emits the modern <link rel="icon" type="image/png"> tag', () => { | ||
| expect(markup).toMatch(/rel=["']icon["']\s+type=["']image\/png["']/); | ||
| }); | ||
|
|
||
| it('points the modern <link rel="icon"> at _favicon.png', () => { | ||
| // The surrounding <link ...> may embed templating tokens like | ||
| // <%= context%> so we don't try to match the whole tag — we | ||
| // just assert _favicon.png is referenced in the file alongside | ||
| // a rel="icon" tag (asserted separately above). | ||
| expect(markup).toContain(expectedHref); |
There was a problem hiding this comment.
Tie _favicon.png to the modern favicon link.
The current assertions can pass if _favicon.png appears elsewhere while the modern <link rel="icon"> points to a different asset. Match a single link tag containing rel, type, and the expected href.
Proposed test tightening
- it('emits the modern <link rel="icon" type="image/png"> tag', () => {
- expect(markup).toMatch(/rel=["']icon["']\s+type=["']image\/png["']/);
- });
-
- it('points the modern <link rel="icon"> at _favicon.png', () => {
- // The surrounding <link ...> may embed templating tokens like
- // <%= context%> so we don't try to match the whole tag — we
- // just assert _favicon.png is referenced in the file alongside
- // a rel="icon" tag (asserted separately above).
- expect(markup).toContain(expectedHref);
+ it('emits the modern <link rel="icon" type="image/png"> tag pointing at _favicon.png', () => {
+ const escapedHref = expectedHref.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+ expect(markup).toMatch(new RegExp(
+ `<link\\b(?=[^>]*\\brel=["']icon["'])(?=[^>]*\\btype=["']image/png["'])`
+ + `(?=[^>]*\\bhref=["'][^"']*${escapedHref}["'])[^>]*>`,
+ ));
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@portals/publisher/src/main/webapp/source/Tests/Unit/Issue4972/favicon.test.js`
around lines 82 - 91, The two tests ("emits the modern <link rel=\"icon\"
type=\"image/png\"> tag" and "points the modern <link rel=\"icon\"> at
_favicon.png") are too loose because they allow `_favicon.png` to appear
anywhere; tighten the second assertion to match a single <link ...> tag that
contains rel="icon", type="image/png", and the expectedHref together. Replace
the toContain(expectedHref) check on markup with a toMatch using a RegExp that
looks for a single <link[^>]* rel=["']icon["'][^>]* type=["']image\/png["'][^>]*
href=["']<expectedHref>["'][^>]*>, and if expectedHref may contain
regex-significant chars create/use an escapeRegExp utility to escape
expectedHref before interpolating into the RegExp.
Comment on lines
+55
to
+80
| function collectLocaleFiles() { | ||
| const portals = ['publisher', 'admin', 'devportal']; | ||
| const out = []; | ||
| portals.forEach((portal) => { | ||
| const dir = path.join(portalsRoot, `${portal}/src/main/webapp/site/public/locales`); | ||
| if (!fs.existsSync(dir)) return; | ||
| fs.readdirSync(dir).forEach((f) => { | ||
| if (f.endsWith('.json')) { | ||
| out.push({ portal, locale: f, file: path.join(dir, f) }); | ||
| } | ||
| }); | ||
| }); | ||
| return out; | ||
| } | ||
|
|
||
| describe('Issue #4972 — locale files for Base.Footer.Footer.product_details', () => { | ||
| const year = expectedYear(); | ||
| const localeFiles = collectLocaleFiles(); | ||
|
|
||
| it('the publisher defaultMessage exposes an expected year', () => { | ||
| expect(year).toMatch(/^\d{4}$/); | ||
| expect(Number(year)).toBeGreaterThanOrEqual(2026); | ||
| }); | ||
|
|
||
| it('locale directories were discovered', () => { | ||
| expect(localeFiles.length).toBeGreaterThan(0); |
There was a problem hiding this comment.
Fail when an expected portal locale directory is missing.
Line 60 silently skips missing portal locale dirs, and Line 80 only proves that at least one locale file exists. That can drop admin/devportal coverage without failing this regression test.
Proposed test hardening
+const PORTALS = ['publisher', 'admin', 'devportal'];
+
function collectLocaleFiles() {
- const portals = ['publisher', 'admin', 'devportal'];
const out = [];
- portals.forEach((portal) => {
+ PORTALS.forEach((portal) => {
const dir = path.join(portalsRoot, `${portal}/src/main/webapp/site/public/locales`);
- if (!fs.existsSync(dir)) return;
+ if (!fs.existsSync(dir)) {
+ throw new Error(`Missing locale directory for ${portal}: ${dir}`);
+ }
fs.readdirSync(dir).forEach((f) => {
if (f.endsWith('.json')) {
out.push({ portal, locale: f, file: path.join(dir, f) });
}
});
@@
it('locale directories were discovered', () => {
expect(localeFiles.length).toBeGreaterThan(0);
+ expect(new Set(localeFiles.map(({ portal }) => portal))).toEqual(new Set(PORTALS));
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function collectLocaleFiles() { | |
| const portals = ['publisher', 'admin', 'devportal']; | |
| const out = []; | |
| portals.forEach((portal) => { | |
| const dir = path.join(portalsRoot, `${portal}/src/main/webapp/site/public/locales`); | |
| if (!fs.existsSync(dir)) return; | |
| fs.readdirSync(dir).forEach((f) => { | |
| if (f.endsWith('.json')) { | |
| out.push({ portal, locale: f, file: path.join(dir, f) }); | |
| } | |
| }); | |
| }); | |
| return out; | |
| } | |
| describe('Issue #4972 — locale files for Base.Footer.Footer.product_details', () => { | |
| const year = expectedYear(); | |
| const localeFiles = collectLocaleFiles(); | |
| it('the publisher defaultMessage exposes an expected year', () => { | |
| expect(year).toMatch(/^\d{4}$/); | |
| expect(Number(year)).toBeGreaterThanOrEqual(2026); | |
| }); | |
| it('locale directories were discovered', () => { | |
| expect(localeFiles.length).toBeGreaterThan(0); | |
| const PORTALS = ['publisher', 'admin', 'devportal']; | |
| function collectLocaleFiles() { | |
| const out = []; | |
| PORTALS.forEach((portal) => { | |
| const dir = path.join(portalsRoot, `${portal}/src/main/webapp/site/public/locales`); | |
| if (!fs.existsSync(dir)) { | |
| throw new Error(`Missing locale directory for ${portal}: ${dir}`); | |
| } | |
| fs.readdirSync(dir).forEach((f) => { | |
| if (f.endsWith('.json')) { | |
| out.push({ portal, locale: f, file: path.join(dir, f) }); | |
| } | |
| }); | |
| }); | |
| return out; | |
| } | |
| describe('Issue `#4972` — locale files for Base.Footer.Footer.product_details', () => { | |
| const year = expectedYear(); | |
| const localeFiles = collectLocaleFiles(); | |
| it('the publisher defaultMessage exposes an expected year', () => { | |
| expect(year).toMatch(/^\d{4}$/); | |
| expect(Number(year)).toBeGreaterThanOrEqual(2026); | |
| }); | |
| it('locale directories were discovered', () => { | |
| expect(localeFiles.length).toBeGreaterThan(0); | |
| expect(new Set(localeFiles.map(({ portal }) => portal))).toEqual(new Set(PORTALS)); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@portals/publisher/src/main/webapp/source/Tests/Unit/Issue4972/localeYear.test.js`
around lines 55 - 80, The test silently skips missing portal locale directories
in collectLocaleFiles (portals array in collectLocaleFiles) which can hide
missing admin/devportal coverage; update the test or collectLocaleFiles so it
asserts per-portal presence: for each portal in portals (publisher, admin,
devportal) check the directory at path.join(portalsRoot,
`${portal}/src/main/webapp/site/public/locales`) exists and contains at least
one .json, and fail the test if any portal directory is missing or empty
(reference collectLocaleFiles, portals array, and localeFiles variable to
implement this per-portal existence/coverage check).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes two independent cosmetic UI bugs reported in wso2/api-manager#4972:
<link rel="shortcut icon">tag. Modern browsers prefer<link rel="icon" type="image/png">and some contexts do not reliably fall back to the legacy form, so the browser tab icon did not render. DevPortal already emits both tags; this change mirrors that pattern on Publisher and Admin.defaultMessagestill said© 2025while Publisher and DevPortal were already rolled forward to 2026 in commit d93574b ("Update Footers and Headers"). The Admin JSX was missed in that commit.Issue: wso2/api-manager#4972
Changes
portals/publisher/src/main/webapp/site/public/pages/index.jsp.hbs<link rel="icon" type="image/png" href="<%= context%>/site/public/images/_favicon.png">portals/publisher/src/main/webapp/publisher/index.html<link rel="icon" type="image/png" href="/site/public/images/_favicon.png">(static webpack-dev variant)portals/admin/src/main/webapp/site/public/pages/index.jsp.hbs<link rel="icon" type="image/png" href="<%= context%>/site/public/images/_favicon.png">portals/admin/src/main/webapp/admin/index.ejs<link rel="icon" type="image/png" href="/site/public/images/_favicon.png">portals/admin/src/main/webapp/source/src/app/components/Base/index.jsx© 2025→© 2026in the CopyrightdefaultMessageAsset dependency:
_favicon.pngalready exists alongsidefavicon.pngin all three portals'site/public/images/directories with identical bytes (md50ca49db64a600f55344ab6970935859d). No binary asset change is needed.Verification
Built the rebuilt
admin.warandpublisher.warfrom source, patched them into a freshwso2am-4.7.0pack, and ran a Playwright + curl regression check. Both bugs pass; DevPortal is unchanged.Bug 1 — favicon tags now emitted on all three portals (HTML head):
GET /{portal}/site/public/images/_favicon.pngreturnsHTTP 200 image/png 36131 byteson all three.Bug 2 — Admin footer renders correctly:
Server log: no
ERROR/FATAL/ stack-trace lines after startup.Regression tests
This PR also adds four regression test suites under
portals/publisher/src/main/webapp/source/Tests/Unit/Issue4972/plus an un-skipped rewrite of the PublisherFooter.test.jsx. Tests are colocated under Publisher because the Admin webapp does not currently have a working jest configuration. Coverage:favicon.test.js— asserts all six portal entry templates (index.jsp.hbs+index.html/index.ejsacross Publisher, Admin, DevPortal) emit both<link rel="shortcut icon">and<link rel="icon" type="image/png">referencing_favicon.png, and that the_favicon.pngasset actually exists in every portal'ssite/public/images/directory.copyrightYear.test.js— derives the expected year from Publisher'sFooter.jsxdefaultMessage(known-correct reference) and asserts the Admin and DevPortalBase/index.jsxCopyright components carry the same year. Explicit negative assertion on© 2025.localeYear.test.js— scans every portal'slocales/*.jsonfor theBase.Footer.Footer.product_detailskey and asserts any non-empty translation carries the current year.Footer.test.jsx— un-skipped; asserts the rendered footer text viaIntlProvider+ThemeProvider.Regression-catches-the-bug check: reverting the fix and re-running the tests produces
2 suites failed / 6 tests failed— precisely the six assertions that act as regression guards. Restoring the fix returns the suite to48 passed / 48 total.Test plan
admin.war+publisher.war, patched into freshwso2am-4.7.0pack, verified via Playwright that both bugs are resolved and DevPortal is unchangedERROR/FATAL/ stack-trace)portals/admin/src/main/webapp/site/public/locales/*.jsonfiles do not still carry© 2025at theBase.Footer.Footer.product_detailskey (the JSX fallback is now 2026 anden.json/fr.jsonin source are already 2026, but other locales were not audited in this change)🤖 Generated with Claude Code