Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add links for Debug UI to GraphiQL #6053

Merged

Conversation

a-limyr
Copy link
Contributor

@a-limyr a-limyr commented Sep 9, 2024

Summary

This feature provides the following new functionality for the debug client:
A new button that when pressed will take the parameters set and create a GraphQL trip query that opens in GraphiQL.
Links in the leg lists will now take you to GraphQL queries in GraphiQL for the different entities.

Issue

This should resolve issue 1. and 2. listen under this: #5330 (comment) issue.

Copy link

codecov bot commented Sep 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.74%. Comparing base (5137512) to head (f68ea6e).
Report is 104 commits behind head on dev-2.x.

Additional details and impacted files
@@            Coverage Diff             @@
##             dev-2.x    #6053   +/-   ##
==========================================
  Coverage      69.73%   69.74%           
- Complexity     17314    17319    +5     
==========================================
  Files           1960     1960           
  Lines          74267    74271    +4     
  Branches        7603     7605    +2     
==========================================
+ Hits           51793    51798    +5     
+ Misses         19831    19830    -1     
  Partials        2643     2643           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@a-limyr a-limyr marked this pull request as ready for review September 9, 2024 13:00
@a-limyr a-limyr requested a review from a team as a code owner September 9, 2024 13:00
@testower testower added the OTP Debug UI The OTP bundled client label Sep 9, 2024
@leonardehrenfried leonardehrenfried changed the title Debug graphiql interaction Add links for Debug UI to GraphiQL Sep 10, 2024
@a-limyr
Copy link
Contributor Author

a-limyr commented Sep 10, 2024

I have refactored and created the following elements:

  • ItineraryGraphiQLLineLink
  • ItineraryGraphiQLQuayLink
  • ItineraryGraphiQLAuthorityLink

I have also added the functionality to click on authority to create a graphQL query.

@leonardehrenfried
Copy link
Member

I will let @testower do the proper review but I was thinking of a very generic component like:

<GraphiQLLink query={query} args={args} text={leg.name}/>

@testower
Copy link
Contributor

What @leonardehrenfried said is also what I had in mind. The text can also just be the child(ren) of the component like i suggested initially.

@testower
Copy link
Contributor

Actually, what Leonard suggested is even more sophisticated.

@a-limyr
Copy link
Contributor Author

a-limyr commented Sep 11, 2024

Ok, the idea behind creating different link types where to remove the need for the encapsulating element from fetching the different query types (line, quay etc.), but maybe adding type as an argument and make the link element handle the query logic (matching type with query). This will result in something like this:

<GraphiQLLink args={args} text={leg.name}/>

Would that be ok?

@leonardehrenfried
Copy link
Member

How would that component figure out the correct query to use?

@a-limyr
Copy link
Contributor Author

a-limyr commented Sep 11, 2024

With use of a QueryType variable, it can either be part of args:

<GraphiQLLink args={args} text={leg.name}/>

or as a separat argument:

<GraphiQLLink args={args} type={type} text={leg.name}/>

Any preference for an approach?

@leonardehrenfried
Copy link
Member

Not really. @testower might have an opinion.

@testower
Copy link
Contributor

No strong preference. To be perfectly honest, I think the current solution works well enough that we don't have to spend more time on it now. Maybe in the future if we add more graphiql-links we could revisit this topic.

Copy link
Member

@leonardehrenfried leonardehrenfried left a comment

Choose a reason for hiding this comment

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

If it's good enough for testower, it's good enough for me.

@leonardehrenfried leonardehrenfried merged commit c141a7c into opentripplanner:dev-2.x Sep 11, 2024
6 checks passed
t2gran pushed a commit that referenced this pull request Sep 11, 2024
@t2gran t2gran added this to the 2.6 milestone Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OTP Debug UI The OTP bundled client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants