Skip to content

feat(web): implemented step 1 tutor onboarding #522

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

mostafakamar2308
Copy link
Member

@mostafakamar2308 mostafakamar2308 commented Mar 19, 2025

Quality Checklist

  • I took the time to compare my implementation with the design.
  • I ensured that the this pull request satisfies all the requirements in the associated task.
  • I performed a self-review.

ClickUp

@neuodev
Copy link
Member

neuodev commented Mar 19, 2025

🚀 WEB
🚀 UI
🚀 LANDING
🚀 DASHBOARD

@neuodev
Copy link
Member

neuodev commented Mar 19, 2025

🚀 WEB
🚀 UI
🚀 LANDING
🚀 DASHBOARD

@neuodev
Copy link
Member

neuodev commented Mar 19, 2025

🚀 WEB
🚀 UI
🚀 LANDING
🚀 DASHBOARD

@mostafakamar2308 mostafakamar2308 self-assigned this Mar 19, 2025
Comment on lines 100 to 103
interviewer_name: string | null;
interviewee_name: string | null;
interviewer_image: string | null;
interviewee_image: string | null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

By convention, the Row type reflects the data model in the database. So, I think we should define another extended type for this use case just to preserve the convention. (we may call it PopulatedRow to follow the codebase: see topic/room/rating in the types package).

The first thing popped up in my head, after reading this, that we are storing interviewer(ee)_name(image) in the interviews table, which would be terrible!

export type PopulatedRow = Row & {
  interviewer_name: string | null;
  interviewee_name: string | null;
  interviewer_image: string | null;
  interviewee_image: string | null;
}

@@ -44,13 +44,63 @@ export type Self = {
updatedAt: string;
};

export type FullInterview = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just to follow the codebase. We may call it PopulatedSelf. And moreover, simply define it as follow:

Despite repeating the id info. This is more readable and consistent.

export type PopulatedSelf = Self & {
  /**
   * tutor manager details
   */
  interviewer: {
    id: number;
    name: string | null;
    image: string | null;
    feedback: string | null;
  };
  /**
   * tutor details
   */
  interviewee: {
    id: number;
    name: string | null;
    image: string | null;
    feedback: string | null;
  };
};

Copy link
Collaborator

Choose a reason for hiding this comment

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

After modifying the types as I suggested. Leave the old functionality as it is. And define/implement a new function, lets call it findPopulatedInterviews, that returns the populated type PopulatedSelf.

item: T
): IAvailabilitySlot.SubSlot {
export function asSubSlot<
T extends ILesson.Self | (IInterview.Self | IInterview.FullInterview),
Copy link
Collaborator

Choose a reason for hiding this comment

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

ILesson.Self | IInterview.Self should be sufficient. As the IInterview.PopulatedSelf extends IInterview.Self.

Comment on lines 155 to 156
const { list: userInterviews, total } = await interviews.find({
users: query.users,
meta: query.meta,
statuses: query.statuses,
Copy link
Collaborator

Choose a reason for hiding this comment

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

As I mentioned before. Lets just keep the old implementation as it is, and extend models/interviews functionality by adding a new function: findPopulatedInterviews.

"tutor.onboarding.tutor-info.form.topics.loading": "برجاء الانتظار... جاري تحميل مجالات الخبرة!",
"tutor.onboarding.tutor-info.form.topics.error": "عذرًا، حدث خطأ أثناء تحميل مجالات الخبرة. برجاء المحاولة مرة أخرى",
"tutor.onboarding.tutor-info.form.error": "عذرًا، حدث خطأ أثناء حفظ التعديلات. برجاء المحاولة مرة أخرى",
"tutor.onboarding.tutor-info.form.next": "التالي",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I prefer general and short texts like this be stored in the "labels..." scope.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think "انضم الآن", "ابدأ الآن", and "التالي" should be stored with keys in the labels scope: "lables.join-now", etc.
What do you think?

@neuodev
Copy link
Member

neuodev commented Mar 20, 2025

🚀 WEB
🚀 UI
🚀 LANDING
🚀 DASHBOARD

@mostafakamar2308 mostafakamar2308 force-pushed the mk/book-interview branch 6 times, most recently from 0bc49a8 to 7003a2e Compare March 26, 2025 10:01
@neuodev
Copy link
Member

neuodev commented Mar 26, 2025

🚀 WEB
🚀 UI
🚀 LANDING
🚀 DASHBOARD

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.

3 participants