source-academy / frontend

Frontend of Source Academy, an online experiential environment for computational thinking (React, Redux, Saga, Blueprint)
https://sourceacademy.org
Apache License 2.0
103 stars 167 forks source link

Migrate to Redux Toolkit part 6 #2952

Closed RichDom2185 closed 6 months ago

RichDom2185 commented 7 months ago

Description

Now that we are using the builder pattern, we can more easily split the code for better clarity and also better enforce some separation.

Type of change

How to test

Checklist

coveralls commented 7 months ago

Pull Request Test Coverage Report for Build 8911594148

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
src/commons/achievement/AchievementView.tsx 0 1 0.0%
src/commons/workspace/WorkspaceReducer.ts 6 7 85.71%
src/pages/academy/grading/subcomponents/GradingWorkspace.tsx 0 1 0.0%
src/commons/sagas/BackendSaga.ts 2 4 50.0%
src/features/playground/PlaygroundReducer.ts 4 7 57.14%
src/commons/mocks/BackendMocks.ts 0 4 0.0%
src/commons/workspace/reducers/editorReducer.ts 136 143 95.1%
src/commons/application/reducers/SessionsReducer.ts 17 29 58.62%
src/commons/workspace/reducers/cseReducer.ts 5 21 23.81%
src/features/stories/StoriesReducer.ts 2 35 5.71%
<!-- Total: 243 323 75.23% -->
Files with Coverage Reduction New Missed Lines %
src/commons/workspace/WorkspaceReducer.ts 2 79.87%
src/features/stories/StoriesReducer.ts 2 13.95%
<!-- Total: 4 -->
Totals Coverage Status
Change from base Build 8798229512: -0.03%
Covered Lines: 5299
Relevant Lines: 14893

💛 - Coveralls
JoelChanZhiYang commented 7 months ago

It looks good! The naming of "multiFileReducer" might be a bit confusing to people newer to Source Academy. I would not explicitly separate the multi-file and the single file (since the single file is simply a subset of the multi-file).

I would just call it the editorReducer.

Some parts of the multiFileReducer would be good contenders for a future refactor. It seem strange that cursor location is stored in global state.

chownces commented 6 months ago

Debugged and fixed the failing tests following the RTK migration as discussed offline with @RichDom2185

Analysis:

Solution:

Misc.