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

Feat/Fix: Add Prospects to Subcourse #560

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open

Conversation

realmayus
Copy link
Contributor

@realmayus realmayus commented Jun 6, 2024

…pects,

prevent user from changing email to itself (violates uniqueness constraint in DB)
@Jonasdoubleyou Jonasdoubleyou temporarily deployed to user-app-feat-add-prosp-zfashv June 6, 2024 13:03 Inactive
@Jonasdoubleyou Jonasdoubleyou temporarily deployed to user-app-feat-add-prosp-nfznjf June 6, 2024 13:21 Inactive
src/pages/single-course/ProspectList.tsx Outdated Show resolved Hide resolved
src/pages/ChangeEmail.tsx Outdated Show resolved Hide resolved
@Jonasdoubleyou
Copy link
Member

(would prefer one change per PR)

@realmayus realmayus changed the title Feat/Fix: Add Prospects to Subcourse & Prevent Invalid Email Changes Feat/Fix: Add Prospects to Subcourse Oct 10, 2024
Copy link
Contributor

@JeangelLF JeangelLF left a comment

Choose a reason for hiding this comment

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

(This can be tackled in a separate task) I noticed a small, unexpected behavior:

When a pupil clicks on Kursleiter:in kontaktieren, they are immediately marked as a prospect on the CoursePage, even if they haven't sent a message yet. This feels a bit unexpected (At least for me 😅)

Could we:

  • Filter out empty chat prospects: Only show users as prospects once they've sent a message.
  • Add a confirmation modal: Inform the user that they’ll be added as a prospect after confirming.

Other than that, it looks good, thank you! 💯

)}
{showWaitingListProspectListTab && (
<TabsContent value="prospect-list">
<WaitingListProspectList
Copy link
Contributor

@JeangelLF JeangelLF Oct 28, 2024

Choose a reason for hiding this comment

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

We could also tackle this in a separate task:

It would be nice to include a button to open the chat with a specific prospect.

We just need to add a prop to WaitingListProspectList

onContact?: (pupilId: string) => void

and pass it to the ParticipantRow (which already renders a chat button).
Then, on the SingleCourseStudent, we just navigate to that conversation.
What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice idea! I'll implement it here.

@realmayus
Copy link
Contributor Author

How could we implement filtering out the empty chats?
Either, we could

  • fetch the chat data from TalkJS on every call of the resolver, or
  • the backend could set a flag in the prospect chat DB entity
    • for that, the backend would have to be notified when the first message was sent in a chat (possibly via a TalkJS webhook?).

I think the latter should be our preferred approach for performance reasons.
What do you think? Did I miss any more obvious solutions?

@JeangelLF
Copy link
Contributor

he backend could set a flag in the prospect chat DB entity
for that, the backend would have to be notified when the first message was sent in a chat (possibly via a TalkJS webhook?).

That one makes sense. But now that I think about it twice, even if we filter out empty chats, the flow still would be a bit weird. It would be something like:

1. SuS clicks on Kursleiter kontaktieren
2. SuS sends whatever message on the chat (under the hood, this already marks the user as a prospect)
3. HuH adds the user to the course
4. SuS gets a notification that they joined the course (???)

This feels a bit off. I think we should just add a modal informing the user about what happens if they proceed to the chat.
What do you think? 👀

@realmayus
Copy link
Contributor Author

I think this does make sense, as HuH know that prospects are interested users that contacted them via chat. We should change the notfication action though, good idea. Then we can have something like "Max has added you to course X"

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