WikiEducationFoundation / WikiEduDashboard

Wiki Education Foundation's Wikipedia course dashboard system
https://dashboard.wikiedu.org
MIT License
388 stars 627 forks source link

Identify the most confusing code in the codebase #5299

Open ragesoss opened 1 year ago

ragesoss commented 1 year ago

Set a timer for yourself, and spend 1 hour browsing through the codebase and trying to understand how it works, what the code you are exploring does, and how it relates to the rest of the system. As you go, note individual files or sections of code that you find particularly confusing.

At the end of the hour, post a comment here noting one or more files that you think are especially confusing or poorly implemented, along with any ideas you have for improving them (whether through refactoring, adding comments, adding documentation, or something else).

If the spirit moves you, you can also attempt to fix some of the problems you found and open a PR, or open an issue with a detailed description that would help someone else attempt to fix the problems.

saha23s commented 1 year ago

@ragesoss I have explored the codebase in the big picture. I have an intro knowledge on React and Ruby on Rails, never used them together. I don't understand how the .haml files are related to .jsx files and building the frontend. I wish there were more comments in the .js and .jsx files to say which functions are doing what. Some .js files like article_finder_action.js has commented out codes too. I want to know the best practices .haml or .jsx files should have and contribute to them if possible. But I am a little confused where to start commenting - can you help me providing some resources to know how the basic infrastructure is working?

ragesoss commented 1 year ago

I can give you a quick overview...

Typically, an HTML request will hit a Rails controller and it will return an HTML page built from .haml templates. A shared .haml template (_head.html.haml) will add the main JS files to the page. One template will add a #nav_root element to the page, and another template will (for many routes) add a #react_root element to the page. The JS bundle's entry point (main.js) will load app.jsx, which will look for the two root elements, and render the Nav and Main React components. When those components render, they will fire off JSON requests (depending on the current route) in order to fetch data from the server. Those JSON requests will hit a Rails controller, and render a jbuilder template to return JSON data, which the React app will then use to re-render.

justiceotuya commented 1 year ago

My worry is that redux is managing both server and client state, leading to

I am suggesting that we use react query to help with managing server state.

i will like to lead this initiative @ragesoss

ragesoss commented 1 year ago

My worry is that redux is managing both server and client state

I'm not sure what you mean by this. We only use Redux client-side on the frontend.

justiceotuya commented 1 year ago

My worry is that redux is managing both server and client state

I'm not sure what you mean by this. We only use Redux client-side on the frontend.

export const fetchCampaigns = courseId => (dispatch) => { return ( fetchCampaignsPromise(courseId) .then((data) => { dispatch({ type: RECEIVE_COURSE_CAMPAIGNS, data }); }) .catch(response => (dispatch({ type: API_FAIL, data: response }))) ); };

this code is taken from campaign_actions.js where we fetch campaign promise from the server and save it in a redux object tree before using the data. so everytime we fetch data from the server we save via redux before using that data.

server state is data gotten from the server via a fetch request while client state are those created ourselves in the component.

This kind of code is handled very well by react query and would help us delete lots of redux boilerplates codes

ragesoss commented 1 year ago

Thanks for explaining. That's not something I'm very worried about, but if you can show what a minimal implementation would look like, I that would give me a better idea of whether it makes sense to work on.

HARSHITHASALADI1 commented 10 months ago

Could you please assign me this issue?