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.

Sign up for GitHub

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

Jump to bottom

Remove legacyURLs in getGraphQLFragments() #384

Closed
chillu opened this issue Jun 1, 2021 · 2 comments
Closed

Remove legacyURLs in getGraphQLFragments() #384

chillu opened this issue Jun 1, 2021 · 2 comments
Assignees
@GuySartorelli
Labels
impact/low type/api-break type/enhancement
Milestone
Silverstripe CMS 5.3

Comments

@chillu
Copy link
Member

chillu commented Jun 1, 2021 •
edited by emteknetnz
Loading

We shouldn't support legacy accumulated during an alpha stage

Acceptance criteria

  • References to legacy url and graphql legacy are removed from the codebase
  • All our GraphQL logic is still functional.
  • This change is made in the CMS 5 branches.

Note

We don't think this will break anything in CMS 5 ... but if it does, just re-target the change to CMS 6 instead.

Shared CI run

Failures in shared CI run are existing, I've replicated them all running locally with a fresh 5.x-dev sink install

  • vendor/silverstripe/cms/tests/behat/features/insert-a-link.feature:35
  • vendor/silverstripe/cms/tests/behat/features/insert-a-link.feature:78
  • vendor/silverstripe/cms/tests/behat/features/insert-anchor-link.feature:33
  • vendor/silverstripe/elemental-bannerblock/tests/Behat/features/block-link-field.feature:18

PRs
@chillu chillu added this to the 4.0.0 milestone Jun 1, 2021
chillu added a commit to open-sausages/silverstripe-graphql that referenced this issue Jun 1, 2021
@chillu
Use public/_graphql/ subfolder
edba2a9 
This is consistent with public/_resources which is also auto-generated by the CMS.
It enables sysadmins to lock down write access to public/ during runtime, with this potentially granular exception.
And it makes it easier to discover which part of the system is responsible for writing these files.

The change is backwards compatible with existing file locations,
although we probably want to remove that before going stable:
silverstripe#384

Relies on equivalent PR to silverstripe/admin.

The docs are already stating this new location so this was more of an oversight:
https://docs.silverstripe.org/en/4/developer_guides/graphql/getting_started/building_the_schema/#viewing-the-generated-code
@chillu chillu mentioned this issue Jun 1, 2021
Merged
unclecheese pushed a commit that referenced this issue Nov 4, 2021
@chillu
Use public/_graphql/ subfolder (#385)
64d8d46 
This is consistent with public/_resources which is also auto-generated by the CMS.
It enables sysadmins to lock down write access to public/ during runtime, with this potentially granular exception.
And it makes it easier to discover which part of the system is responsible for writing these files.

The change is backwards compatible with existing file locations,
although we probably want to remove that before going stable:
#384

Relies on equivalent PR to silverstripe/admin.

The docs are already stating this new location so this was more of an oversight:
https://docs.silverstripe.org/en/4/developer_guides/graphql/getting_started/building_the_schema/#viewing-the-generated-code
@maxime-rainville
Copy link
Contributor

I had a look and we still have a bunch of graphQL legacy stuff in the codebase.

I don't know that we have to wait for CMS 6 to remove an of this stuff, we've already nerfed GraphQL v3 compatibility in CMS 5. If someone tried to start using "legacy" mod in CMS5 nothing would work for them.

https://github.com/silverstripe/silverstripe-admin/blob/a3aa41cea4c4df82050eef65ad5efcfae7bfde69/client/src/boot/apollo/getGraphqlFragments.js#L24-L48

image

@GuySartorelli GuySartorelli changed the title Remove legacyURLs in getGraphQLFragments() before v4 stable Remove legacyURLs in getGraphQLFragments() May 22, 2024
@emteknetnz emteknetnz self-assigned this Jun 18, 2024
This was referenced Jun 18, 2024
Merged
Merged
Merged
Merged
@emteknetnz emteknetnz removed their assignment Jun 20, 2024
@GuySartorelli
Copy link
Member

PRs merged

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

@GuySartorelli GuySartorelli

Labels
impact/low type/api-break type/enhancement
Projects
None yet
Milestone
Silverstripe CMS 5.3
Development

No branches or pull requests
4 participants
@chillu @maxime-rainville @emteknetnz @GuySartorelli