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

828 steamline win level info message #873

Merged
merged 16 commits into from
Mar 27, 2024

Conversation

pmarsh-scottlogic
Copy link
Contributor

@pmarsh-scottlogic pmarsh-scottlogic commented Mar 21, 2024

Description

Before, when the user would send a message that would cause the level to be won, we would check if wonLevel in the backend's response was true, and if so, we would add a congratulations message to the chat History, causing an extra API call.

Now, we return the congratulations messagge (wonLevelMessage) in the chat response. The front end simply appends it to the history if it is there, and can use this to determine if the user has won the level.

Screenshot

image

Notes

also does these things:

  • renames the chatMessageType LEVEL_INFO to LEVEL_COMPLETE
  • removes the information header from the congratulations message. Adds party poppers instead

Concerns

  • Now in the backend, the type ChatHttpResponse contains the wonLevel boolean property. It gets marked as true at the points of the send email function call. At the end of handleChatToGPT in the chatController, we check wonLevel. If we have won the level, then we add the congratulations message to the history and return it to the frontend. The frontend only needs the congratualtions message to determine that a level has been won, making wonLevel redundant in the frontend. This is untidy, but not necessarily problematic. This will be made irrelevant by Refactor: Shift the logic for checking win condition #780

Checklist

Have you done the following?

  • Linked the relevant Issue
  • Added tests
  • Ensured the workflow steps are passing

Copy link
Member

@chriswilty chriswilty left a comment

Choose a reason for hiding this comment

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

Minor suggestions only, and one to chew on.

backend/test/unit/controller/chatController.test.ts Outdated Show resolved Hide resolved
frontend/src/components/ChatBox/ChatBox.tsx Outdated Show resolved Hide resolved
frontend/src/components/ChatBox/ChatBox.tsx Show resolved Hide resolved
Copy link
Member

@chriswilty chriswilty left a comment

Choose a reason for hiding this comment

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

Lovely!

@pmarsh-scottlogic
Copy link
Contributor Author

I need to solve a problem where we are not sending the congratulations message if there is an OpenAI error after the winning email has been sent

@pmarsh-scottlogic
Copy link
Contributor Author

I've decided the gain is not worth the cost. In the startlingly unlikely event that the user would win a level with a sent email, but open has an error after the function call, then the user will not win the level.

@pmarsh-scottlogic pmarsh-scottlogic merged commit b231883 into dev Mar 27, 2024
4 checks passed
@pmarsh-scottlogic pmarsh-scottlogic deleted the 828-steamline-win-level-info-message branch March 27, 2024 13:51
chriswilty pushed a commit that referenced this pull request Apr 8, 2024
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.

Stop adding messages to backend history via frontend 🛠🛠
2 participants