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

Discuss Refactoring of API in Client #39590

Closed
5 tasks
ShaunSHamilton opened this issue Sep 16, 2020 · 7 comments
Closed
5 tasks

Discuss Refactoring of API in Client #39590

ShaunSHamilton opened this issue Sep 16, 2020 · 7 comments
Labels
platform: api Server application that needs familiarity with Express, Loopback, MongoDB etc. scope: tools/scripts Scripts for supporting dev work, generating config and build artifacts, etc. status: discussing Under discussion threads. Closed as stale after 60 days of inactivity.

Comments

@ShaunSHamilton
Copy link
Member

ShaunSHamilton commented Sep 16, 2020

This comes about as I was going to refactor Certification.js to not rely on hardcoded values, but after discussing with Mrugesh and Oliver, realise it is not a quick section to fix.

Top sections to refactor:

  • ./redux Move from using both epics and sagas to just sagas.

What I have been looking into:

  • Certification.js (certMapSelector, and renderLegacyFullStack)
  • ShowCertification.js (imports certMap)
  • certAndProjectMap.js (Kris left a comment about refactoring to automatically generate from md/.json files)
  • getChallenges.js (Oliver mentioned splitting this up into more bite-sized functions to allow better access outwith)

Minutes:

  • Rather than moving away from Redux, it would be beneficial to leverage it without complex epics
  • Use Redux hooks, for refactor of functional components

  • certAndProjectMap.js is likely to be deleted
  • certMap and projectMap should be fetched with useStaticQuery when we build the client
  • Certification.js and ShowCertification.js could make use of a graphQL query to fetch the certMap

Feel free to edit this. @ojeytonwilliams and @raisedadead feel free to correct/add anything I missed.

@ShaunSHamilton ShaunSHamilton added status: discussing Under discussion threads. Closed as stale after 60 days of inactivity. platform: api Server application that needs familiarity with Express, Loopback, MongoDB etc. status: waiting triage This issue needs help from moderators and users to reproduce and confirm its validity and fix. scope: tools/scripts Scripts for supporting dev work, generating config and build artifacts, etc. labels Sep 16, 2020
@RandellDawson
Copy link
Member

Would we want to do something about utils/block-nameify.js as it relates to hardcoded names?

@ojeytonwilliams
Copy link
Contributor

certMap and projectMap can be built during the build of the challenge files in getChallenges.js

That was just a bad idea of mine. Mrugesh was right: if we can, we should just make use of useStaticQuery to fetch the data we need when we build the client. That creates a better separation of the client and the curriculum.

@ojeytonwilliams
Copy link
Contributor

Would we want to do something about utils/block-nameify.js as it relates to hardcoded names?

Not necessarily, since the names have to appear somewhere. That said, it's possible the best approach is to just apply blockNameify to set the superBlocks while building the curriculum and then fetch them with a GraphQL query in the client. Same reasoning as above.

@RandellDawson
Copy link
Member

RandellDawson commented Sep 16, 2020

@ojeytonwilliams Ultimately, I just want the process of adding a new block (project in v7) easier. Currently, this involves updating multiple files in various files located in the client and curriculum folders. There is a lot of duplicate information in those various files.

@ojeytonwilliams
Copy link
Contributor

@RandellDawson since you're doing this a lot, would you mind noting all the places you have to change things? Not necessarily right now, but it would be great if you could keep this in mind next time you add a block.

@RandellDawson
Copy link
Member

@ojeytonwilliams Yes, I have been meaning to document it anyway.

@raisedadead raisedadead removed the status: waiting triage This issue needs help from moderators and users to reproduce and confirm its validity and fix. label Apr 29, 2021
@moT01
Copy link
Member

moT01 commented Aug 3, 2021

Thank you for reporting this issue.

This is a standard message notifying you that the issue has become stale, so we are going to close it.

If you think we're wrong in closing this issue, please request for it to be reopened. Thank you and happy coding.

@moT01 moT01 closed this as completed Aug 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: api Server application that needs familiarity with Express, Loopback, MongoDB etc. scope: tools/scripts Scripts for supporting dev work, generating config and build artifacts, etc. status: discussing Under discussion threads. Closed as stale after 60 days of inactivity.
Projects
None yet
Development

No branches or pull requests

5 participants