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.

Sign up for GitHub

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

Jump to bottom

Adding message and chat HTML #100

Merged
merged 2 commits into from
May 26, 2022
Merged

Adding message and chat HTML #100

merged 2 commits into from
May 26, 2022

Conversation

Yuval-Vino
Copy link
Collaborator

@Yuval-Vino Yuval-Vino commented May 1, 2022 •
edited
Loading

Desired Outcome

Adding chat feature to Hotails

Implemented Changes

Added message and chat html
updated message views in order to fit the HTML files
updated navbar chat button to redirect to message HTML
updated hotails urls added message and chat htmls
updated function to daycare model - get_daycare_primary_image_url (as implemnted in #89 for showing daycare picture in chat)
also added CSS for the relevant htmls

Connected Issue/Story

Resolves #99

Changelog

  • The CHANGELOG has been updated
  • This PR does not include user-facing changes and doesn't require a
    CHANGELOG update

Test coverage

Added test for the HTML's that checking that we are in the right site
test_shown_only_relevant_contacts
test_shown_only_relevant_message_in_chat
test_send_new_message
test_cant_send_blank_message

Documentation

  • Docs (e.g. READMEs) were updated in this PR
  • A follow-up issue to update official docs has been filed here: insert issue ID
  • This PR does not require updating any documentation
Adding pictures of the url

before picking specifc user to talk

07174943-e82d-4df8-b131-7e900eb3ca22

after choosing chat (as a dogowner)

58c7d26e-be18-4d03-9df8-bd2ae6b0eef2

after choosing chat (as daycare)

21730a1c-eaa7-47c9-beb3-4193a8b29a44

ErezCohenn
ErezCohenn reviewed May 1, 2022
View reviewed changes
Copy link
Collaborator

@ErezCohenn ErezCohenn left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Niceee @Yuval-Vino
don't forget to assign yourself, and add ready for review label,
and the most important tests are missing

@Yuval-Vino Yuval-Vino self-assigned this May 1, 2022
@Yuval-Vino
Copy link
Collaborator Author

Niceee @Yuval-Vino don't forget to assign yourself, and add ready for review label, and the most important tests are missing

Done!
Added test

this PR is ready for reviews!!

@Yuval-Vino Yuval-Vino requested review from ormergi and Omeramsc May 2, 2022 19:36
ErezCohenn
ErezCohenn requested changes May 3, 2022
View reviewed changes
Copy link
Collaborator

@ErezCohenn ErezCohenn left a comment

Choose a reason for hiding this comment

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

left some comments below, @Yuval-Vino take a look and tell me what you think

daycare/models.py Outdated Show resolved Hide resolved
message/test_html.py Outdated Show resolved Hide resolved
@Omeramsc
Copy link
Contributor

Omeramsc commented May 3, 2022

Please lint and re-orgenize message/templates/messages.html, there is a mess in the tags there (closing tags with no opening tags, and seems like divs that are not closed either? hard to read)

@Yuval-Vino
Copy link
Collaborator Author

left some comments below, @Yuval-Vino take a look and tell me what you think

thanks for the reviews, I address them all :)

@Yuval-Vino
Copy link
Collaborator Author

Please lint and re-orgenize message/templates/messages.html, there is a mess in the tags there (closing tags with no opening tags, and seems like divs that are not closed either? hard to read)

Done! also addressed all of your comments , let me know if you see more possible changes to improve

@Yuval-Vino Yuval-Vino requested review from ErezCohenn and Omeramsc May 3, 2022 09:33
ErezCohenn
ErezCohenn approved these changes May 3, 2022
View reviewed changes
Copy link
Collaborator

@ErezCohenn ErezCohenn left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM !
don't forget to rebase.
also I think you should squash the commit Fixing the PR after comments to Adding message and chat HTML

OfirMatasas
OfirMatasas approved these changes May 3, 2022
View reviewed changes
Copy link
Collaborator

@OfirMatasas OfirMatasas left a comment

Choose a reason for hiding this comment

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

Looks amazing! great job, @Yuval-Vino

tamirmatok
tamirmatok approved these changes May 4, 2022
View reviewed changes
Copy link
Collaborator

@tamirmatok tamirmatok left a comment

Choose a reason for hiding this comment

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

LGTM ! great job @Yuval-Vino .

ormergi
ormergi requested changes May 11, 2022
View reviewed changes
Copy link
Contributor

@ormergi ormergi left a comment

Choose a reason for hiding this comment

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

Hi Yuval, please squash intermediate commits

@Yuval-Vino
Copy link
Collaborator Author

Hi Yuval, please squash intermediate commits

Done

@Yuval-Vino Yuval-Vino requested a review from ormergi May 16, 2022 11:35
ormergi
ormergi requested changes May 25, 2022
View reviewed changes
Copy link
Contributor

@ormergi ormergi left a comment

Choose a reason for hiding this comment

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

Hi Yuval,
Please fix the commit message and body.
Also see my inline comments.

main/templates/main/base_template.html Outdated Show resolved Hide resolved
message/test_html.py Outdated Show resolved Hide resolved
message/test_html.py Show resolved Hide resolved
@Yuval-Vino Yuval-Vino requested a review from ormergi May 26, 2022 14:32
@Omeramsc
Copy link
Contributor

Please resolve conflicts ASAP if you want it in for the demo.
There is room for improvements, but It is good enough to get in.

@Yuval-Vino
Copy link
Collaborator Author

Please resolve conflicts ASAP if you want it in for the demo. There is room for improvements, but It is good enough to get in.

fixed

@Omeramsc
Copy link
Contributor

+1 LGTM

@Omeramsc Omeramsc merged commit 3bd7de8 into redhat-beyond:main May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OfirMatasas OfirMatasas OfirMatasas approved these changes

@ErezCohenn ErezCohenn ErezCohenn approved these changes

@tamirmatok tamirmatok Awaiting requested review from tamirmatok

@jeniaSakirko jeniaSakirko Awaiting requested review from jeniaSakirko

@Omeramsc Omeramsc Awaiting requested review from Omeramsc

@ormergi ormergi Awaiting requested review from ormergi

Assignees

@Yuval-Vino Yuval-Vino

Labels
Ready for review Status/New
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create message and chat HTML
6 participants
@Yuval-Vino @Omeramsc @ormergi @ErezCohenn @OfirMatasas @tamirmatok