Skip to content

Conversation

@A-Wheeto
Copy link
Contributor

@A-Wheeto A-Wheeto commented Feb 12, 2025

Status

Review progress:

  • Browser tested
  • Front-end review completed
  • Tech review completed

What's changed?

  • Added primary computing glossary table component into Strapi via graphql
  • New CMS view component for the table
  • New stubs, mocks and tests

@A-Wheeto A-Wheeto changed the base branch from main to 2935-strapi-graphql February 12, 2025 15:24
@A-Wheeto A-Wheeto temporarily deployed to teachcomputing-pr-2294 February 13, 2025 15:24 Inactive
@A-Wheeto A-Wheeto temporarily deployed to teachcomputing-pr-2294 February 13, 2025 15:25 Inactive
@A-Wheeto A-Wheeto temporarily deployed to teachcomputing-pr-2294 February 14, 2025 15:24 Inactive
@A-Wheeto A-Wheeto temporarily deployed to teachcomputing-pr-2294 February 17, 2025 09:25 Inactive
@A-Wheeto A-Wheeto temporarily deployed to teachcomputing-pr-2294 February 17, 2025 10:40 Inactive
Base automatically changed from 2935-strapi-graphql to main February 18, 2025 09:39
@A-Wheeto A-Wheeto force-pushed the 2956-move-primary-computing-glossary-to-strapi branch from b6d23b8 to 107e47c Compare March 3, 2025 13:49
Copy link
Contributor

@msquance-stem msquance-stem left a comment

Choose a reason for hiding this comment

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

LGTM, just a naming change suggestion, and also a fix for lighthouse

Providers::Strapi::Client.new
when "graphql"
Providers::Strapi::GraphqlClient.new
if Rails.env.test?
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the old version, it should now just be
Providers::Strapi::GraphqlClient.new
We no longer need to pass the schema

@@ -0,0 +1,31 @@
module Cms
module Collections
class PrimaryGlossaryTable < Resource
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should rename this to PrimaryGlossaryTableItems, just to make it clear its separate to the component? I wouldn't bother changing the name on Strapi, just the collection name here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, I will update this.

msquance-stem and others added 12 commits March 4, 2025 14:32
Probably needs a bit of tidying up, but its a working start.
Still need to implement all the different grahql queries
…in some extra testing of the older rest features in case we still need them

Added some changes to the graphql base query to include generation of filter and sort params.

Updated the filter factory to support the rest mode (prepended $ sign) and the graphql mode.
Updated CMS::Resource with all_records method
New primary glossary table collection
Primary glossary table in Dynamic zone
Added mocks
Testing for component
Updated schema
Adding tests for all_records
Updated mocks and tests
Update Providers::Strapi::GraphqlClient to newer version
@A-Wheeto A-Wheeto force-pushed the 2956-move-primary-computing-glossary-to-strapi branch from e1f5c24 to 94db3f8 Compare March 4, 2025 14:38
@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 4, 2025

@tc-deploybot tc-deploybot temporarily deployed to teachcomputing-pr-2294 March 4, 2025 14:53 Inactive
@msquance-stem msquance-stem merged commit 7866c74 into main Mar 12, 2025
8 checks passed
@msquance-stem msquance-stem deleted the 2956-move-primary-computing-glossary-to-strapi branch March 12, 2025 11:56
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.

4 participants