Skip to content

Commit dbb582c

Browse files
committed
fix graphql bug, extend examples
1 parent 50a1db3 commit dbb582c

10 files changed

Lines changed: 170 additions & 47 deletions

File tree

‎runtime/pages/example/index.html‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,15 @@ <h1>Feedback integration example</h1>
1414
<p>This page exercises batched native upvotes, multiple discussion threads, reactions, labels, polls, comments, minimized or deleted comments, replies, accepted answers, and GitHub links.</p>
1515
<p>API: <code id="api-origin"></code></p>
1616
<div id="authentication-status" class="authentication-status"></div>
17+
<h2>Voting and reaction cards</h2>
18+
<p>These cards use one anonymous batched counter request. The final card intentionally has no discussion.</p>
19+
<section id="cards" class="cards"></section>
20+
<h2>Full discussions</h2>
1721
<section id="thread" class="thread"></section>
1822
<form id="comment-form" class="comment-form">
1923
<label>Thread <select id="comment-key"></select></label>
2024
<textarea id="comment-body" rows="4" maxlength="16000" placeholder="Add a comment"></textarea>
21-
<button type="submit">Sign in and comment</button>
25+
<button id="comment-submit" type="submit">Sign in and comment</button>
2226
<button id="clear-reply" type="button" hidden>Cancel reply</button>
2327
</form>
2428
<p id="status" role="status">Loading…</p>

‎runtime/pages/example/style.css‎

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,26 +15,45 @@ main {
1515
}
1616

1717
.authentication-status {
18-
align-items: center;
19-
background: #f1f5f8;
20-
border: 1px solid #d7dbe0;
2118
display: flex;
22-
gap: 0.75rem;
23-
justify-content: space-between;
19+
justify-content: flex-end;
2420
margin: 1rem 0;
25-
padding: 0.75rem;
2621
}
2722

28-
.authentication-status small {
29-
color: #555;
30-
margin-right: auto;
23+
.feedback-authentication-dot {
24+
background: #c9362b;
25+
border: 2px solid #8f241d;
26+
border-radius: 50%;
27+
height: 1.25rem;
28+
padding: 0;
29+
width: 1.25rem;
30+
}
31+
32+
.feedback-authentication-dot.authenticated {
33+
background: #2b9b4b;
34+
border-color: #17632e;
3135
}
3236

3337
.thread {
3438
display: grid;
3539
gap: 1rem;
3640
}
3741

42+
.cards {
43+
display: grid;
44+
gap: 1rem;
45+
grid-template-columns: repeat(auto-fit, minmax(14rem, 1fr));
46+
margin-bottom: 2rem;
47+
}
48+
49+
.ranking-card {
50+
border: 1px solid #d7dbe0;
51+
border-radius: 0.5rem;
52+
padding: 1rem;
53+
}
54+
55+
.ranking-card h3 { margin-top: 0; }
56+
3857
.discussion { border: 1px solid #ddd; border-radius: 0.5rem; padding: 1rem; }
3958
.own-post { border-color: #356fc0; box-shadow: inset 4px 0 #356fc0; }
4059
.controls { display: flex; flex-wrap: wrap; gap: 0.5rem; margin-top: 1rem; }
@@ -51,6 +70,12 @@ main {
5170
margin-top: 1rem;
5271
}
5372

73+
.reaction-control {
74+
border: 1px solid #c7ced8;
75+
border-radius: 0.25rem;
76+
padding: 0.35rem 0.55rem;
77+
}
78+
5479
.poll-option {
5580
border: 1px solid #d7dbe0;
5681
margin-top: 0.5rem;

‎runtime/src/auth/status.ts‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,22 +22,21 @@ export function createAuthenticationStatus(
2222
): AuthenticationStatusController {
2323
const element = document.createElement("section");
2424
element.className = "feedback-authentication-status";
25-
const message = document.createElement("span");
2625
const action = document.createElement("button");
2726
action.type = "button";
28-
element.append(message, action);
27+
action.className = "feedback-authentication-dot";
28+
element.append(action);
2929
options.mount.replaceChildren(element);
3030

3131
const refresh = (): void => {
3232
const token = options.authentication.token();
3333
const signedIn = token !== null;
34-
message.textContent = signedIn
34+
const status = signedIn
3535
? (options.signedInLabel?.(token.viewerId) ?? "Authenticated")
3636
: (options.signedOutLabel ?? "Not authenticated");
37-
action.textContent = signedIn
38-
? (options.logoutLabel ?? "Log out")
39-
: (options.loginLabel ?? "Log in");
40-
action.setAttribute("aria-label", action.textContent);
37+
action.classList.toggle("authenticated", signedIn);
38+
action.title = `${status}. ${signedIn ? (options.logoutLabel ?? "Log out") : (options.loginLabel ?? "Log in")}`;
39+
action.setAttribute("aria-label", action.title);
4140
action.onclick = () => {
4241
if (options.authentication.token() !== null) {
4342
options.authentication.clear();
@@ -59,11 +58,7 @@ export function createAuthenticationStatus(
5958
};
6059
};
6160

62-
if (options.explanation) {
63-
const explanation = document.createElement("small");
64-
explanation.textContent = options.explanation;
65-
element.insertBefore(explanation, message);
66-
}
61+
if (options.explanation) element.title = options.explanation;
6762
refresh();
6863
return { element, refresh };
6964
}

‎runtime/src/example/page.ts‎

Lines changed: 87 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ import type { ReactionState } from "../api/client.js";
33
import { trustedOrigin } from "../auth/origin.js";
44
import { Authentication } from "../auth/controller.js";
55
import { createAuthenticationStatus } from "../auth/status.js";
6-
import { viewerSubjectStates } from "../protocol/github.js";
7-
import type { ViewerSubjectState } from "../protocol/github.js";
6+
import { setReaction, viewerSubjectStates } from "../protocol/github.js";
7+
import type { Reaction, ViewerSubjectState } from "../protocol/github.js";
88

99
const parameters = new URLSearchParams(location.search);
1010
const site = parameters.get("site") ?? "feedback-cpp-social";
@@ -21,13 +21,14 @@ const client = new FeedbackClient({ apiOrigin, site });
2121
const authentication = new Authentication({ site, callbackOrigin: location.origin, service: client });
2222
const status = required("status");
2323
let replyTo: { key: string; id: string } | undefined;
24+
let cardViewerStates = new Map<string, ViewerSubjectState>();
2425
required("api-origin").textContent = apiOrigin;
2526
const authenticationStatus = createAuthenticationStatus({
2627
mount: required("authentication-status"),
2728
authentication,
2829
explanation: "Sign in is required to add comments or replies.",
2930
onError: (error) => { status.textContent = error instanceof Error ? error.message : "Authentication failed."; },
30-
onChange: () => { void render(); },
31+
onChange: () => { updateAuthenticationUi(); void render(); },
3132
});
3233
required("comment-form").addEventListener("submit", (event) => {
3334
event.preventDefault();
@@ -44,6 +45,7 @@ for (const key of keys) {
4445
option.textContent = key;
4546
commentKey.append(option);
4647
}
48+
updateAuthenticationUi();
4749
void render();
4850

4951
async function submitComment(): Promise<void> {
@@ -53,6 +55,7 @@ async function submitComment(): Promise<void> {
5355
let token = authentication.token();
5456
token ??= await authentication.authenticate();
5557
authenticationStatus.refresh();
58+
updateAuthenticationUi();
5659
const key = replyTo?.key ?? commentKey.value;
5760
await client.addComment(key, textarea.value, token, replyTo?.id);
5861
textarea.value = "";
@@ -70,12 +73,26 @@ async function render(): Promise<void> {
7073
try {
7174
let states = await client.reactions(keys);
7275
const token = authentication.token();
73-
if (token !== null) states = await client.syncViewerUpvotes(keys, token);
76+
if (token !== null) {
77+
states = await client.syncViewerUpvotes(keys, token);
78+
const ids = [...states.values()].flatMap((state) => state.id ? [state.id] : []);
79+
cardViewerStates = new Map(await viewerSubjectStates(token, ids));
80+
} else {
81+
cardViewerStates.clear();
82+
}
83+
renderRankingCards(states);
7484
const existingKeys = keys.filter((key) => states.get(key)?.id);
75-
const contents = await client.discussionContents(existingKeys);
7685
const root = required("thread");
7786
root.replaceChildren();
78-
await Promise.all(keys.map(async (key) => {
87+
let contents = new Map<string, { discussion: Record<string, unknown> }>();
88+
let contentError: unknown;
89+
try {
90+
contents = new Map(await client.discussionContents(existingKeys));
91+
await addViewerState([...contents.values()].map((value) => value.discussion));
92+
} catch (error) {
93+
contentError = error;
94+
}
95+
for (const key of keys) {
7996
const article = document.createElement("article");
8097
article.className = "discussion";
8198
root.append(article);
@@ -86,20 +103,77 @@ async function render(): Promise<void> {
86103
}
87104
try {
88105
const payload = contents.get(key);
89-
if (!payload) throw new Error("Missing discussion content");
90-
await addViewerState(payload.discussion);
106+
if (!payload) {
107+
throw contentError instanceof Error
108+
? contentError
109+
: new Error("Missing discussion content");
110+
}
91111
renderDiscussion(article, key, payload.discussion, state);
92112
} catch (error) {
93113
appendText(article, "h2", key);
94114
appendText(article, "p", error instanceof FeedbackError ? error.code : "Unable to load thread.");
95115
}
96-
}));
97-
status.textContent = "Ready.";
116+
}
117+
status.textContent = contentError === undefined ? "Ready." : "Counters loaded; discussions unavailable.";
98118
} catch (error) {
99119
status.textContent = error instanceof FeedbackError ? error.code : "Unable to load discussions.";
100120
}
101121
}
102122

123+
function renderRankingCards(states: ReadonlyMap<string, ReactionState>): void {
124+
const root = required("cards");
125+
root.replaceChildren();
126+
for (const key of keys) {
127+
const state = states.get(key);
128+
const card = document.createElement("article");
129+
card.className = "ranking-card";
130+
appendText(card, "h3", key);
131+
const button = actionButton(
132+
`${state?.viewerHasUpvoted === true ? "Remove upvote" : "Upvote"} · ${String(state?.upvotes ?? 0)}`,
133+
() => upvote(key),
134+
);
135+
button.setAttribute("aria-pressed", String(state?.viewerHasUpvoted === true));
136+
card.append(button);
137+
const reactions = document.createElement("div");
138+
reactions.className = "reactions";
139+
for (const [name, count] of Object.entries(state?.reactions ?? {})) {
140+
const selected = state?.id
141+
? cardViewerStates.get(state.id)?.reactions.has(name as Reaction) === true
142+
: false;
143+
const reaction = actionButton(
144+
`${name}: ${String(count)}`,
145+
() => react(key, name as Reaction, selected),
146+
);
147+
reaction.className = "reaction-control";
148+
reaction.setAttribute("aria-pressed", String(selected));
149+
reactions.append(reaction);
150+
}
151+
card.append(reactions);
152+
if (!state?.id) appendText(card, "small", "No discussion yet; all counters are zero.");
153+
root.append(card);
154+
}
155+
}
156+
157+
async function react(key: string, reaction: Reaction, selected: boolean): Promise<void> {
158+
try {
159+
const token = authentication.token() ?? await authentication.authenticate();
160+
authenticationStatus.refresh();
161+
updateAuthenticationUi();
162+
const state = (await client.reactions([key])).get(key);
163+
if (!state?.id) throw new Error("Create the discussion with an upvote before reacting.");
164+
await setReaction(token, state.id, reaction, !selected);
165+
status.textContent = `Updated ${reaction} on ${key}.`;
166+
await render();
167+
} catch (error) {
168+
status.textContent = error instanceof Error ? error.message : "Unable to react.";
169+
}
170+
}
171+
172+
function updateAuthenticationUi(): void {
173+
const authenticated = authentication.token() !== null;
174+
required("comment-submit").textContent = authenticated ? "Comment" : "Sign in and comment";
175+
}
176+
103177
function renderDiscussion(
104178
root: HTMLElement,
105179
key: string,
@@ -241,7 +315,7 @@ function renderComment(key: string, comment: Record<string, unknown>, depth: num
241315
const tag = appendText(
242316
article,
243317
"span",
244-
`${string(value.content)}: ${String(integer(record(value.users)?.totalCount))}`,
318+
`${string(value.content)}: ${String(integer(record(value.reactors)?.totalCount))}`,
245319
"tag",
246320
);
247321
if (value.viewerHasReacted === true) tag.classList.add("selected");
@@ -285,11 +359,11 @@ function isViewer(author: unknown): boolean {
285359
return viewerId !== undefined && string(record(author)?.id) === viewerId;
286360
}
287361

288-
async function addViewerState(content: Record<string, unknown>): Promise<void> {
362+
async function addViewerState(contents: readonly Record<string, unknown>[]): Promise<void> {
289363
const token = authentication.token();
290364
if (token === null) return;
291365
const subjects = new Map<string, Record<string, unknown>>();
292-
collectSubjects(content, subjects);
366+
for (const content of contents) collectSubjects(content, subjects);
293367
const states = await viewerSubjectStates(token, [...subjects.keys()]);
294368
for (const [id, state] of states) overlayViewerState(subjects.get(id), state);
295369
}

‎src/feedback/protocol/github/client.py‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,11 @@ async def graphql(
9292
if response.status_code >= 400:
9393
raise _response_error(response, "github_http_error")
9494
if data.get("errors"):
95+
logger.warning(
96+
"GitHub GraphQL operation rejected: errors=%s github_request_id=%s",
97+
_safe_graphql_errors(data.get("errors")),
98+
response.headers.get("x-github-request-id"),
99+
)
95100
raise _response_error(response, "github_graphql_error")
96101
result = data.get("data")
97102
if not isinstance(result, dict):
@@ -184,6 +189,26 @@ def _json_object(response: httpx.Response) -> dict[str, Any]:
184189
return value
185190

186191

192+
def _safe_graphql_errors(value: object) -> str:
193+
if not isinstance(value, list):
194+
return "unknown"
195+
result: list[str] = []
196+
for error in value[:5]:
197+
if not isinstance(error, dict):
198+
result.append("unknown")
199+
continue
200+
kind = error.get("type")
201+
path = error.get("path")
202+
safe_kind = kind if isinstance(kind, str) and kind.isidentifier() else "unknown"
203+
safe_path = (
204+
".".join(item for item in path if isinstance(item, str))
205+
if isinstance(path, list)
206+
else ""
207+
)
208+
result.append(f"{safe_kind}:{safe_path}" if safe_path else safe_kind)
209+
return ",".join(result) or "unknown"
210+
211+
187212
def _response_error(
188213
response: httpx.Response, code: str, *, retry_after: float | None = None
189214
) -> GitHubError:

‎src/feedback/protocol/github/discussions.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ async def create(
8989
return discussion
9090

9191
async def content(
92-
self, site: SiteConfig, discussion_ids: list[str], comments: int = 100
92+
self, site: SiteConfig, discussion_ids: list[str], comments: int = 25
9393
) -> dict[str, Any]:
9494
return await self._client.graphql(
9595
site.installation_id,
@@ -155,8 +155,8 @@ def _vote_counts(value: object) -> tuple[int, int, dict[str, int]]:
155155
for group in value:
156156
if not isinstance(group, dict) or not isinstance(group.get("content"), str):
157157
raise GitHubError("github_malformed_response")
158-
users = group.get("users")
159-
count = users.get("totalCount") if isinstance(users, dict) else None
158+
reactors = group.get("reactors", group.get("users"))
159+
count = reactors.get("totalCount") if isinstance(reactors, dict) else None
160160
if isinstance(count, bool) or not isinstance(count, int) or count < 0:
161161
raise GitHubError("github_malformed_response")
162162
counts[group["content"]] = count

‎src/feedback/protocol/github/queries/discussion.graphql‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ query FeedbackDiscussion(
2424
repository { id }
2525
category { id name }
2626
labels(first: 100) @include(if: $includeLabels) { nodes { id name color description } }
27-
reactionGroups @include(if: $includeReactions) { content users { totalCount nodes { id } } }
27+
reactionGroups @include(if: $includeReactions) { content reactors { totalCount } }
2828
upvoteCount
2929
poll @include(if: $includePolls) {
3030
question
@@ -44,7 +44,7 @@ query FeedbackDiscussion(
4444
isMinimized @include(if: $includeModeration)
4545
minimizedReason @include(if: $includeModeration)
4646
isAnswer @include(if: $includeAnswers)
47-
reactionGroups @include(if: $includeCommentReactions) { content users { totalCount } }
47+
reactionGroups @include(if: $includeCommentReactions) { content reactors { totalCount } }
4848
upvoteCount @include(if: $includeCommentUpvotes)
4949
replies(first: $comments) {
5050
totalCount
@@ -57,7 +57,7 @@ query FeedbackDiscussion(
5757
createdAt updatedAt deletedAt
5858
isMinimized @include(if: $includeModeration)
5959
minimizedReason @include(if: $includeModeration)
60-
reactionGroups @include(if: $includeCommentReactions) { content users { totalCount } }
60+
reactionGroups @include(if: $includeCommentReactions) { content reactors { totalCount } }
6161
upvoteCount @include(if: $includeCommentUpvotes)
6262
}
6363
}

‎src/feedback/protocol/github/queries/discussion_reactions.graphql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ query FeedbackDiscussionReactions($ids: [ID!]!, $includeReactions: Boolean!) {
77
upvoteCount
88
reactionGroups @include(if: $includeReactions) {
99
content
10-
users { totalCount nodes { id } }
10+
reactors { totalCount }
1111
}
1212
}
1313
}

0 commit comments

Comments
 (0)