Skip to content
This repository was archived by the owner on Mar 27, 2025. It is now read-only.

chore: implemented expand QPs for showcase#52

Open
sanderPostma wants to merge 11 commits intodevelopfrom
feature/SHOWCASE-159
Open

chore: implemented expand QPs for showcase#52
sanderPostma wants to merge 11 commits intodevelopfrom
feature/SHOWCASE-159

Conversation

@sanderPostma
Copy link
Contributor

No description provided.

"scripts": {
"start": "ts-node src/index.ts",
"dev": "ts-node-dev src/index.ts",
"dev": "ts-node-dev --transpile-only src/index.ts",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolves 3min startup time

@sanderPostma sanderPostma marked this pull request as ready for review March 14, 2025 09:58

@Get('/')
public async getAll(): Promise<ShowcasesResponse> {
public async getAll(@QueryParam('expand') expand?: ShowcaseExpand[]): Promise<ShowcasesResponse> {
Copy link
Contributor

Choose a reason for hiding this comment

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

i do not see id's being returned. now it is returning empty arrays

Copy link
Contributor Author

Choose a reason for hiding this comment

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

open 14-03


@Get('/:slug')
public async getOne(@Param('slug') slug: string): Promise<ShowcaseResponse> {
public async getOne(@Param('slug') slug: string, @QueryParam('expand') expand?: ShowcaseExpand[]): Promise<ShowcaseResponse> {
Copy link
Contributor

Choose a reason for hiding this comment

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

i do not see id's being returned. now it is returning empty arrays

Copy link
Contributor Author

Choose a reason for hiding this comment

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

open 14-03

const getResponse = await request.get(`/showcases/${createdShowcase.slug}`).expect(200)

// Verify no related entities are expanded
expect(getResponse.body.showcase.scenarios).toEqual([])
Copy link
Contributor

Choose a reason for hiding this comment

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

should we not check on id's here?

// Create a properly typed array for scenarios
const scenariosArray: Scenario[] = []

for (const scenarioJoin of result.scenarios) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we not use a .map?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

if (expandSet.has(ShowcaseExpand.PERSONAS) && 'personas' in result && Array.isArray(result.personas)) {
const personasArray: Persona[] = []

for (const personaJoin of result.personas) {
Copy link
Contributor

Choose a reason for hiding this comment

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

use a .map?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

credentialDefinition: {
with: {
icon: true,
...(expandSet.has(ShowcaseExpand.ASSET_CONTENT) ? { icon: true } : {}),
Copy link
Contributor

Choose a reason for hiding this comment

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

...(expandSet.has(ShowcaseExpand.ASSET_CONTENT) && { icon: true })

Copy link
Contributor

Choose a reason for hiding this comment

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

same for all the others

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

const scenarioItems = scenariosMap.get(showcaseData.id) || []
const scenariosArray: Scenario[] = []

for (const scenarioJoin of scenarioItems) {
Copy link
Contributor

Choose a reason for hiding this comment

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

use a .map?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed


public getShowcases = async (): Promise<Showcase[]> => {
return this.showcaseRepository.findAll()
public getShowcases = async (expand?: ShowcaseExpand[]): Promise<Showcase[]> => {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we create an params object for these functions? this extends much nicer in the future

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

export const normalizeExpandParams = (expand?: string[]): ShowcaseExpand[] => {
const expandMap: Record<string, ShowcaseExpand> = {
scenarios: ShowcaseExpand.Scenarios,
credentialdefinitions: ShowcaseExpand.CredentialDefinitions,
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we have these double? with snake case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes it gives some upper/lower/snake case flexibility

@Brummos
Copy link
Contributor

Brummos commented Mar 14, 2025

Also tests are failing, run pnpm test from the root

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants