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

Documentation enhancement #30

Open
florentianayuwono opened this issue Jul 1, 2022 · 15 comments
Open

Documentation enhancement #30

florentianayuwono opened this issue Jul 1, 2022 · 15 comments
Assignees
Labels
documentation Improvements or additions to documentation
Milestone

Comments

@florentianayuwono
Copy link
Owner

Gonna work on adding comments to the newly-added code now :) expects questions from me haha

@florentianayuwono florentianayuwono added the documentation Improvements or additions to documentation label Jul 1, 2022
@florentianayuwono florentianayuwono added this to the Milestone 3 milestone Jul 1, 2022
@florentianayuwono florentianayuwono self-assigned this Jul 1, 2022
@florentianayuwono
Copy link
Owner Author

@KamiliArsyad is there any difference when using req.body.user_id and req.user.user_id? I just notice that we often have some code that use body and then there is also user. If there is actually no difference, then we can just decide on one convention whether to call it body or user and I could make the changes.

@KamiliArsyad
Copy link
Collaborator

Yup they're totally different.
It has something to do with the fact that some routes are protected.
Protected Routes
Request with a **token** -> pass through _authMiddleware_ where we add a _.user_ that contains the user data as an element of request -> we use that req.user inside the controller that handles the data.
To get a hang of this, you can also access other fields such as email, phoneNumber, etc. when we access req.user that was added by authMiddleware.

Unprotected Routes that Involves req.body.user_id
Request with a user_id (and possibly other fields) element inside the body -> we use that body.user_id inside the controller to get the user_id.
As you can see, in this scenario, the request itself contains the user_id in its body and hence we access that user_id by simply accessing the body.

We don't use the req.body.user_id thing in protected routes simply because we already have an id just from the authentication token and thus we don't require user_id as a field in the request's body.

Hope this helps :)

@florentianayuwono
Copy link
Owner Author

@KamiliArsyad for this code in Login.jsx,

useEffect(() => {
setMessage(user.message);

return () => setMessage("");
}, [user.message]);

is this like setting the message to the message from the authentication process (loginUser) and then emptied it again? I am not really sure how to explain it.

@KamiliArsyad
Copy link
Collaborator

The return clause in React's useEffect is a cleanup function i.e. a function that we want to execute once we are done with the component (unmount the component) and thus we want to clear the message back to an empty string every time we are unmounting the component.

@florentianayuwono
Copy link
Owner Author

Thank you! Okay, so after it setMessage to user.message, it then clean it up again by making setMessage to empy string, correct? But then what is exactly user.message referring to? Is it really the message returned by the authentication process? (I am not sure but seems like authentication render error message when login is unsuccessful, right?)

@KamiliArsyad
Copy link
Collaborator

Okay, so after it setMessage to user.message, it then clean it up again by making setMessage to empy string, correct?

You can think of it more or less that way yeah. I believe clean up functions are also executed right before the effect is being done.

But then what is exactly user.message referring to?

user is an object that represents the authentication state of the user. You can think of it as a global authentication variable that can be accessed by any component that subscribes to the authContext using our custom useAuthContext hook and is a descendant of the AuthProvider component (we can see in the RouteManager file that every page is a descendant of the AuthRoute, thus every component in every page has access to the value provided by the context). The value provided by the authentication context are specified in the authProvider file: the state, user, and the dispatcher, dispatch (poor naming system yes because every feature has the same dispatcher name; we shall change that later).

The authReducer file is where the initial state and all the possible state modifications are listed. You can see that the authentication object (one that is being "broadcasted" to the whole project) has a message property that could sometimes be filled with the error message it might get when authServices is making a request to the API. In short, yes, it's the message returned by the authentication process.

You can view the context and reducer documentation if you wish to learn more. We're basically replacing redux by creating our own state management system using these powerful hooks (useContext and useReducer) because I figure out it's a good chance to learn everything from the ground up for these things.

note: I believe the codes I made for the pages to handle the error messages are still quite poor because I haven't had the time to improve on it, that's why I decided to make a bare-minimum working code that displays the raw messages from the API straight to the user when there's an error. This shouldn't be the final code and should you think there's a better way to handle the messages on the pages component, feel free to mess around with stuff.

@florentianayuwono
Copy link
Owner Author

Thank you! Also, there is a warning in my VSC saying that this message in Dashboard.jsx:

const isSubmitted: boolean
React Hook useEffect has a missing dependency: 'nav'. Either include it or remove the dependency array.eslint react-hooks/exhaustive-deps

I have tried "npm i nav" but the warning persists. Do you know what's wrong?

@KamiliArsyad
Copy link
Collaborator

It's because we use the nav function which exists outside of the effect. The recommended practice is to make each useEffect hooks independent and list each of its dependency on the array of the second argument. It's a little complicated to fix but we we should be able to clear this warning later using useCallback (I believe Dan Abramov has written an extensive article about this so don't worry we can refer to it later to fix).

@KamiliArsyad
Copy link
Collaborator

Here's the link to the article.

@florentianayuwono
Copy link
Owner Author

Inside the DisplayBusinessParticular.jsx, what does this code mean?

   {/* right side of the screen (details)*/}
    <div
      className={
        selectedBusiness !== "No selected business"
          ? "col-md-8"
          : "container"
      }
    >
      <div className="container">
        <Outlet context={[selectedBusiness]} />
      </div>
    </div>
  </div>

Is it referring to the thing that happen when you click the business then suddenly the details pop up beside them on the right side?

@KamiliArsyad
Copy link
Collaborator

Yup. Since there will most likely be a change there, I've been thinking on how to modify that part since the past few days so for now just think of that as an incomplete code

@florentianayuwono
Copy link
Owner Author

Inside the businessController, what exactly does this dateParser do?

const dateRangeParser = (date_range) => {
  switch (date_range) {
    case 0:
      return 86400000;
    case 1:
      return 604800000;
    case 2:
      return 2592000000;
    case 3:
      return 31536000000;
    default:
      return 86400000;
  }
};

const dateMinusDateRange = new Date(
  new Date(getDate()) - dateRangeParser(date_range)
);

@KamiliArsyad
Copy link
Collaborator

The date_range is a categorical data, and we want to convert that into miliseconds so that we can use that for working with the Date object

@florentianayuwono
Copy link
Owner Author

So the case 0, 1, 2, 3 is referring to the daily, weekly, monthly and yearly, right? The number is from the 60 * 60 * 24 * number of days. But where is the 0000 ... behind comes from? And why do we need to subtract the date with the number from date parser?

Also, what is the difference between bestCategoricalPlatform and bestPlatformForBusinessCategory? I think bestPlatformForBusinessCategory function is more well-crafted, but are they intended to do different things?

@KamiliArsyad
Copy link
Collaborator

The Date object is interacting using milliseconds as the unit and hence the extra 000.

As for those two functions, there was a design error; sorry it was my bad to not write any comments there.
I wrote the bestCategoricalPlatform function first. The function takes in a list of all sales filtered by a certain product category, and then returns the best platform that produces the most profit for that product category.

I then decided that it's more relevant to take in the list of sales filtered by a certain business category instead and return the platform that produces the most profit for that business category. However I don't think I want to delete the previous function because it might be useful for future product recommendations and so I just leave it there and named it ambiguously similar :))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants