Skip to content

Conversation

@sayomaki
Copy link
Contributor

Description

This PR attempts to resolve #2633 to disable the routes in the public deployment as stories are not yet available. The fix moves all stories-related routes to under fullAcademy router, instead of being under common routes where they would still be accessible in the public deployment.

Alternatively, adding a environment variable can also be considered instead, to toggle whether stories routes will be enabled (similar to how githubAssessments work right now). If this is the right way going forward, let me know and I will refactor the code accordingly.

Also as a side note, the stories component might also need to updated to have its own router instead, as right now they are statically defined in the main routerConfig file. This will also ease the implementation of #2632 where more fine-grained permissions control can be done within its own router. However, this is outside the scope of this PR and should be in its own standalone update.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Code quality improvements

How to test

Check if the routes are accessible when REACT_APP_USE_BACKEND is true and when is false (fullAcademy mode enabled and disabled).

The code has been tested locally, and confirmed that stories routes will no longer be present.

Checklist

  • I have tested this code
  • I have updated the documentation

@coveralls
Copy link

Pull Request Test Coverage Report for Build 5971634627

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 37.395%

Totals Coverage Status
Change from base Build 5971555367: 0.0%
Covered Lines: 5740
Relevant Lines: 14428

💛 - Coveralls

@RichDom2185 RichDom2185 requested a review from chownces August 25, 2023 05:37
Copy link
Contributor

@chownces chownces left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this! This implementation is okay for now, but we will eventually have a separate public stories backend in the future.

On a side note, we have a Source Academy Dev Team chat to coordinate development work. Would you be interested to join in? Feel free to drop me an email at chowenrong@u.nus.edu!

@chownces chownces merged commit 39dafbf into source-academy:master Aug 25, 2023
RichDom2185 pushed a commit to NUS-CS1101S/cadet-frontend that referenced this pull request Aug 25, 2023
RichDom2185 pushed a commit to NUS-CS1101S/cadet-frontend that referenced this pull request Aug 25, 2023
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.

Disable stories routes in public deployment

3 participants