Skip to content

Addressing PR comments - #781

Merged
benckx merged 19 commits into
masterfrom
copilot/add-admin-feed-view-for-only-manchu-games
Jun 9, 2026
Merged

benckx merged 19 commits into
masterfrom
copilot/add-admin-feed-view-for-only-manchu-games

Conversation

Copilot AI commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI linked an issue Jun 9, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Add admin feed view for Manchu games Add Manchu-only admin game feed Jun 9, 2026
Copilot finished work on behalf of benckx June 9, 2026 06:52
Copilot AI requested a review from benckx June 9, 2026 06:52
suspend fun listLastGames(
limit: Int,
statusToExcludes: List<GameEventType> = listOf(),
variantToInclude: Variant? = null,

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot should be a list that by default include all variants.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated in f07c340: listLastGames now takes variantsToInclude: List<Variant> = Variant.entries, so it defaults to all variants and supports subset filters (Manchu feed now passes listOf(Variant.MANCHU)).

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot drop the test

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 0fe1408: I dropped AdminFeedServiceTest.

</tbody>
</table>

<h1>Last Manchu Game Updates</h1>

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot make separate feed page

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 0fe1408 (with follow-up polish in d975c2f): Manchu games now have a separate page at /admin/manchu-feeds.

Copilot AI changed the title Add Manchu-only admin game feed Add separate Manchu-only admin game feed page Jun 9, 2026
Copilot finished work on behalf of benckx June 9, 2026 07:13

@benckx benckx left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot we also need to show manchu bot games

get("/list-games") {
requireAdminRole { adminFeedService.listLastGames() }
}
get("/list-manchu-games") {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot Generalize this to "variants" (i.e. non-xiangqi variants)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 30c629f. I generalized the dedicated endpoints and feed logic to variants (non-Xiangqi): /list-variant-games and /list-variant-bot-games, backed by a shared non-Xiangqi variant filter instead of Manchu-only naming.

Copilot AI commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

@copilot we also need to show manchu bot games

Addressed in d3d4e40. The dedicated /admin/manchu-feeds page now also shows Manchu bot games via a new filtered admin endpoint (/api/admin/analytics/list-manchu-bot-games) and an added Manchu bot games table on that page.

Copilot AI changed the title Add separate Manchu-only admin game feed page Add separate Manchu-only admin feeds page (PvP + bot games) Jun 9, 2026
Copilot finished work on behalf of benckx June 9, 2026 07:35
Copilot AI requested a review from benckx June 9, 2026 07:35
Copilot stopped work on behalf of benckx due to an error June 9, 2026 11:21
@Claude
Claude AI requested a review from benckx June 9, 2026 11:21
@Claude Claude AI changed the title Add separate non-Xiangqi variant admin feeds page (PvP + bot games) Addressing PR comments Jun 9, 2026
Copilot stopped work on behalf of benckx due to an error June 9, 2026 11:44
@Claude
Claude AI requested a review from benckx June 9, 2026 11:44
@benckx benckx changed the title Addressing PR comments add admin feed view for only manchu games Jun 9, 2026
Comment thread webapp/src/main/resources/templates/admin/admin_variant_feeds.html
Copilot stopped work on behalf of benckx due to an error June 9, 2026 12:22
@Claude
Claude AI requested a review from benckx June 9, 2026 12:22

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@claude[agent] In fetchGameMetadata, is there a way to re-use the mappers that are defined at the bottom of the file?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The mappers at the bottom (mapPlayerVsBotGameToDto, mapPlayerVsPlayerGameToDto) are designed for list views and populate fields like redPlayerRating, redUserType, isRedOnline, etc., while fetchGameMetadata is for individual game detail and populates analysisStatus, engine, depth, startFen — fields the list mappers don't set.

The mappers also require a pre-computed onlineUserIds: Set<String> (batched from multiple records), which doesn't apply to a single-game lookup.

For the DB case there's no corresponding mapper at all — that logic lives only in fetchGameMetadata.

So no, they can't be reused as-is without restructuring those mappers. Let me know if you'd like me to refactor them to make reuse possible (e.g. make onlineUserIds optional, add the missing fields).

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@claude[agent] I merged this PR. Do this in another PR. Just fill everything that can be filled each time.

@Claude Claude AI changed the title add admin feed view for only manchu games Addressing PR comments Jun 9, 2026
@Claude
Claude AI requested a review from benckx June 9, 2026 15:27
@benckx
benckx merged commit 781dace into master Jun 9, 2026
3 checks passed
@benckx
benckx deleted the copilot/add-admin-feed-view-for-only-manchu-games branch June 9, 2026 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add admin feed view for only manchu games

3 participants