Skip to content

GG-291 Make nodes and totalCount fields optional#376

Open
kennethpl wants to merge 1 commit intomainfrom
GG-291-Make-nodes-and-totalCount-fields-optional
Open

GG-291 Make nodes and totalCount fields optional#376
kennethpl wants to merge 1 commit intomainfrom
GG-291-Make-nodes-and-totalCount-fields-optional

Conversation

@kennethpl
Copy link
Collaborator

No description provided.

@andreahn
Copy link
Collaborator

Kan du ta en rebase? Ser det er konflikter 😄

@kennethpl kennethpl force-pushed the GG-291-Make-nodes-and-totalCount-fields-optional branch from 45a7d08 to 8ca41eb Compare February 27, 2026 14:21
- Made the connection fields 'nodes' and 'totalCount' optional and
  added tests to verify the behavior.

  Note: There are no codegen tests specifically asserting
  inclusion/exclusion of 'nodes', since it doesn't generate a dedicated
  resolver/method and is handled via the same pagination logic as the
  required 'edges' field.
@kennethpl kennethpl force-pushed the GG-291-Make-nodes-and-totalCount-fields-optional branch from 8ca41eb to 4829838 Compare February 27, 2026 14:45
Comment on lines +186 to +190
if (target.hasTotalCountFieldInReturnType()) {
var countFunction = countFunction(objectToCall, method, params, target.hasServiceReference());
return CodeBlock.of(" $N,\n$L,\n$L$L", VAR_PAGE_SIZE, queryFunction, countFunction, transformWrap);
} else {
return CodeBlock.of(" $N,\n$L$L", VAR_PAGE_SIZE, queryFunction, transformWrap);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (target.hasTotalCountFieldInReturnType()) {
var countFunction = countFunction(objectToCall, method, params, target.hasServiceReference());
return CodeBlock.of(" $N,\n$L,\n$L$L", VAR_PAGE_SIZE, queryFunction, countFunction, transformWrap);
} else {
return CodeBlock.of(" $N,\n$L$L", VAR_PAGE_SIZE, queryFunction, transformWrap);
var countFunction = CodeBlock.ofIf(target.hasTotalCountFieldInReturnType(), ",\n$L", () -> countFunction(objectToCall, method, params, target.hasServiceReference());
return CodeBlock.of(" $N,\n$L$L$L", VAR_PAGE_SIZE, queryFunction, countFunction, transformWrap);

.flatMap(obj -> obj.getFields().stream())
.filter(this::isConnectionObject)
.filter(this::hasTotalCountField)
.forEach(field -> field.setHasTotalCountFieldInReturnType(true));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Er litt skeptisk til å gjøre dette mutabelt på denne måten, det burde være nok å sjekke dette på selve connection-typen, men usikker på om det blir mer eller mindre komplekst. hasTotalCountField-metoden lenger nede burde være nok.

@DisplayName("When optional field 'totalCount' is included in the schema for a field having a service and pagination, generate the count method")
void serviceWithPaginationAndAllOptionalFieldsIncluded() {
assertGeneratedContentMatches(
"operation/withPaginationAndAllOptionalFieldsIncluded",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Det burde være nok å sjekke at count metoden kalles slik som i ResolverPaginationTest.

@DisplayName("When optional field 'totalCount' is not included in the schema for a field having a service and pagination, do not generate the count method")
void serviceWithPaginationAndNoOptionalFieldsIncluded() {
assertGeneratedContentMatches(
"operation/withPaginationAndNoOptionalFieldsIncluded",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Samme her, nok å sjekke at den ikke kommer med.

@@ -0,0 +1,9 @@
type CustomerConnection {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Gi den gjerne et annet navn så de ikke risikerer å kollidere med de andre. For eksempel CustomerNoOptionalsConnection osv.

Det eneste som har noe å si er at den slutter på Connection.


import static graphql.util.TraversalControl.CONTINUE;

public class ConnectionFieldFilter implements ModifyingGraphQLTypeVisitor {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Dette tror jeg skal være unødvendig. Vi kan bare la være å legge dem til i typen i transformasjonen som lager det, hvis ikke den bruker noe ferdigbakt. Det blir uannsett enklere å modifisere og teste typene der enn å kjøre en hel filtrering for å få dem bort igjen

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.

3 participants