Skip to content

Commit 304ff34

Browse files
neo-opus-adatobiu
andauthored
fix(portal): render bot/app avatar glyph in news summary list (#12317) (#12318)
* fix(portal): render bot/app avatar glyph in news summary list (#12317) #12316 routed the timeline HTML avatars through getAvatarHtml (sized <img> for normal users, Font Awesome GitHub glyph for bot/app actors). But the "On this page" summary is a separate consumer: the parsers publish a record `image` URL into the sections store, and Neo.app.content.SectionsList renders `record.image` as a plain <img> — so bot/app actors (whose <login>.png 404s) still rendered a broken image in the summary while the timeline showed the glyph. Close the consumer-shape gap, keeping the bot decision centralized: - Portal.view.content.Component: extract isBotActor(user) (single source of truth for the bot/app check, shared by getAvatarHtml + the new helper); add getAvatarRecordProps(user) → {image: getAvatarUrl(user)} for normal users, {iconCls: 'fa-brands fa-github'} for bots. - tickets / pulls / discussions parsers: spread ...getAvatarRecordProps(user) into the timeline records (replacing the raw image: getAvatarUrl), so the published record carries iconCls for bot/app actors. - Neo.app.content.SectionsList: add a backward-compatible `iconCls` render branch (generic; no GitHub-specific knowledge — existing image/icon branches untouched). Covers all three news views (AC4); SectionsList stays generic (AC3). Co-Authored-By: neo-opus-4-7 <neo-opus-4-7@neomjs.com> * fix(portal): declare iconCls on TimelineSection model so it survives hydration (#12317) --------- Co-authored-by: tobiu <tobiasuhlig78@gmail.com>
1 parent ce8a2ee commit 304ff34

7 files changed

Lines changed: 86 additions & 10 deletions

File tree

apps/portal/model/TimelineSection.mjs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ class TimelineSection extends ContentSection {
2020
}, {
2121
name: 'icon',
2222
type: 'String'
23+
}, {
24+
name: 'iconCls',
25+
type: 'String'
2326
}, {
2427
name: 'image',
2528
type: 'String'

apps/portal/view/content/Component.mjs

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,18 @@ class Component extends ContentComponent {
7272
return `${this.repoUserUrl}${user}.png?size=40`
7373
}
7474

75+
/**
76+
* @summary True for GitHub bot/app actors whose `github.com/<login>.png` avatar 404s.
77+
*
78+
* Single source of truth for the bot/app actor decision, shared by `getAvatarHtml` (timeline HTML)
79+
* and `getAvatarRecordProps` (summary list), so the bot list is never duplicated in a consumer renderer.
80+
* @param {String} user GitHub login.
81+
* @returns {Boolean}
82+
*/
83+
isBotActor(user) {
84+
return botActors.has(user) || user.endsWith('[bot]')
85+
}
86+
7587
/**
7688
* @summary Renders a bounded avatar for a timeline actor.
7789
*
@@ -82,13 +94,27 @@ class Component extends ContentComponent {
8294
* @returns {String}
8395
*/
8496
getAvatarHtml(user) {
85-
if (botActors.has(user) || user.endsWith('[bot]')) {
97+
if (this.isBotActor(user)) {
8698
return `<i class="neo-timeline-avatar-icon fa-brands fa-github" role="img" aria-label="${user}"></i>`
8799
}
88100

89101
return `<img src="${this.getAvatarUrl(user)}" alt="${user}" loading="lazy">`
90102
}
91103

104+
/**
105+
* @summary Resolves the avatar fields for a timeline entry record, consumed by both the timeline and
106+
* the `Neo.app.content.SectionsList` "On this page" summary.
107+
*
108+
* Normal users get a bounded `image` URL; bot/app actors get an `iconCls` (Font Awesome GitHub glyph)
109+
* instead — so the summary list renders the same no-network glyph the timeline does, rather than a
110+
* broken `<img>` for a 404ing bot avatar. The bot decision stays centralized via `isBotActor`.
111+
* @param {String} user GitHub login.
112+
* @returns {Object} `{image}` for normal users, `{iconCls}` for bot/app actors.
113+
*/
114+
getAvatarRecordProps(user) {
115+
return this.isBotActor(user) ? {iconCls: 'fa-brands fa-github'} : {image: this.getAvatarUrl(user)}
116+
}
117+
92118
/**
93119
* @param {Object} record
94120
* @returns {Promise<void>}

apps/portal/view/news/discussions/Component.mjs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ class Component extends ContentComponent {
200200

201201
me.timelineData.unshift({
202202
id : bodyId,
203-
image: me.getAvatarUrl(data.author),
203+
...me.getAvatarRecordProps(data.author),
204204
name : 'Description',
205205
tag : 'body'
206206
});
@@ -346,7 +346,7 @@ ${fullHtml}
346346

347347
me.timelineData.push({
348348
id,
349-
image: me.getAvatarUrl(comment.user),
349+
...me.getAvatarRecordProps(comment.user),
350350
name : `Comment (${comment.user})`,
351351
tag : 'comment'
352352
});

apps/portal/view/news/pulls/Component.mjs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class Component extends ContentComponent {
5050
cls: ['portal-news-pulls-component'],
5151
/**
5252
* `#N` references in PR bodies are issue numbers → resolve to the portal tickets view.
53-
* Per-content-type config (#12209): the generic content base no longer defaults this.
53+
* Per-content-type config: the generic content base no longer defaults this.
5454
* @member {String} issuesUrl='#/news/tickets/'
5555
*/
5656
issuesUrl: '#/news/tickets/',
@@ -196,7 +196,7 @@ class Component extends ContentComponent {
196196

197197
me.timelineData.push({
198198
id : bodyId,
199-
image: me.getAvatarUrl(author),
199+
...me.getAvatarRecordProps(author),
200200
name : 'Description',
201201
tag : 'body'
202202
});
@@ -213,7 +213,7 @@ class Component extends ContentComponent {
213213

214214
me.timelineData.push({
215215
id,
216-
image: me.getAvatarUrl(entry.user),
216+
...me.getAvatarRecordProps(entry.user),
217217
name : isReview ? `Review (${entry.user})` : `Comment (${entry.user})`,
218218
tag : entry.type
219219
});

apps/portal/view/news/tickets/Component.mjs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ class Component extends ContentComponent {
264264

265265
me.timelineData.unshift({
266266
id : bodyId,
267-
image: me.getAvatarUrl(author),
267+
...me.getAvatarRecordProps(author),
268268
name : 'Description',
269269
tag : 'body'
270270
});
@@ -338,7 +338,7 @@ ${fullHtml}
338338

339339
me.timelineData.push({
340340
id,
341-
image: me.getAvatarUrl(currentUser),
341+
...me.getAvatarRecordProps(currentUser),
342342
name : `Comment (${currentUser})`,
343343
tag : 'comment'
344344
});

src/app/content/SectionsList.mjs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ class SectionsList extends List {
4040

4141
if (record.image) {
4242
content.push({tag: 'img', src: record.image, cls: ['neo-list-icon', 'avatar']})
43+
} else if (record.iconCls) {
44+
// A full Font Awesome class string (e.g. a `fa-brands fa-github` bot glyph) rendered as an
45+
// avatar-sized icon — lets a record supply a no-network glyph instead of an `image` URL.
46+
content.push({tag: 'i', cls: ['neo-list-icon', 'avatar', ...record.iconCls.split(' ')]})
4347
} else if (record.icon) {
4448
content.push({tag: 'i', cls: ['neo-list-icon', 'fa-solid', record.icon]})
4549
}

test/playwright/unit/apps/portal/view/content/Component.spec.mjs

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {test, expect} from '@playwright/test';
1010
import Neo from '../../../../../../../src/Neo.mjs';
1111
import * as core from '../../../../../../../src/core/_export.mjs';
1212
import Component from '../../../../../../../apps/portal/view/content/Component.mjs';
13+
import TimelineSections from '../../../../../../../apps/portal/store/TimelineSections.mjs';
1314

1415
/**
1516
* Unit coverage for the shared avatar helpers on the Portal news timeline base
@@ -19,7 +20,7 @@ import Component from '../../../../../../../apps/portal/view/content/Compo
1920
* no-network glyph. Both helpers are pure (they read only `this.repoUserUrl` / `this.getAvatarUrl`), so
2021
* they are exercised directly on the prototype with a stub context.
2122
*/
22-
const {getAvatarHtml, getAvatarUrl} = Component.prototype;
23+
const {getAvatarHtml, getAvatarRecordProps, getAvatarUrl, isBotActor} = Component.prototype;
2324

2425
test.describe('Portal.view.content.Component — shared avatar helpers', () => {
2526
test('getAvatarUrl bounds the avatar request to ?size=40', () => {
@@ -28,7 +29,7 @@ test.describe('Portal.view.content.Component — shared avatar helpers', () => {
2829
});
2930

3031
test('getAvatarHtml: normal user → sized lazy <img>; bot/app actor → no-network glyph', () => {
31-
const ctx = {repoUserUrl: 'https://github.com/', getAvatarUrl};
32+
const ctx = {repoUserUrl: 'https://github.com/', getAvatarUrl, isBotActor};
3233

3334
// Normal user: bounded (?size=40) lazy <img>, never the full-resolution original.
3435
const normal = getAvatarHtml.call(ctx, 'alice');
@@ -43,5 +44,47 @@ test.describe('Portal.view.content.Component — shared avatar helpers', () => {
4344

4445
// `[bot]`-suffixed app actor: same no-network fallback.
4546
expect(getAvatarHtml.call(ctx, 'some-app[bot]')).toContain('fa-github')
47+
});
48+
49+
test('isBotActor flags known bot/app actors, not normal users', () => {
50+
expect(isBotActor('dependabot')).toBe(true);
51+
expect(isBotActor('github-actions')).toBe(true);
52+
expect(isBotActor('some-app[bot]')).toBe(true);
53+
expect(isBotActor('alice')).toBe(false)
54+
});
55+
56+
test('getAvatarRecordProps: normal user → {image:?size=40}; bot/app actor → {iconCls: github glyph}', () => {
57+
// Drives the summary-list (SectionsList) avatar shape: a normal user yields a bounded image URL,
58+
// a bot/app actor yields a glyph class so the summary renders the no-network glyph, not a 404 <img>.
59+
const ctx = {repoUserUrl: 'https://github.com/', getAvatarUrl, isBotActor};
60+
61+
expect(getAvatarRecordProps.call(ctx, 'alice')).toEqual({image: 'https://github.com/alice.png?size=40'});
62+
expect(getAvatarRecordProps.call(ctx, 'dependabot')).toEqual({iconCls: 'fa-brands fa-github'})
63+
})
64+
});
65+
66+
/**
67+
* The summary list (`Neo.app.content.SectionsList`) renders `record.iconCls` for bot/app actors. That
68+
* only works if `iconCls` is a declared field on the `Portal.model.TimelineSection` contract — an
69+
* undeclared field is dropped during record hydration, so the glyph would silently vanish from the
70+
* summary even though the parser emitted it. This pins the field on the model.
71+
*/
72+
test.describe('Portal.model.TimelineSection — iconCls hydration contract', () => {
73+
test('iconCls survives store hydration as a declared field, alongside image', () => {
74+
const store = Neo.create(TimelineSections, {
75+
id : 'portal-timeline-iconcls-hydration-test',
76+
data: [
77+
{id: 'bot', iconCls: 'fa-brands fa-github'},
78+
{id: 'human', image : 'https://github.com/alice.png?size=40'}
79+
]
80+
});
81+
82+
try {
83+
// An undeclared field would be undefined here → the summary bot-glyph would silently break.
84+
expect(store.get('bot').iconCls).toBe('fa-brands fa-github');
85+
expect(store.get('human').image).toBe('https://github.com/alice.png?size=40')
86+
} finally {
87+
store.destroy()
88+
}
4689
})
4790
});

0 commit comments

Comments
 (0)