Skip to content

Conversation

@lk316
Copy link
Contributor

@lk316 lk316 commented Jul 10, 2025

@lk316 lk316 requested a review from Slashek July 10, 2025 20:00
Copy link
Contributor

@Slashek Slashek left a comment

Choose a reason for hiding this comment

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

Do not invoke GraphQL from partials

assign other_participants_ids = participants_ids | subtract_array: current_participants_ids
assign from_profile = other_participants_ids | first
if from_profile
graphql from_profile = 'modules/profile/profiles/search', id: from_profile
Copy link
Contributor

Choose a reason for hiding this comment

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

please do not invoke GraphQL from partials - all data should be fetched in the controller (Page)

render 'modules/common-styling/user/avatar', size: 's', name: name, imageSrc: from_profile.avatar.photo.versions.sm
endif
print name | raw_escape_string
else # conversation was just initiated
Copy link
Contributor

Choose a reason for hiding this comment

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

to avoid this big if statement with a lot of duplication, can we just fetch and prepare data in Page?

@lk316
Copy link
Contributor Author

lk316 commented Jul 15, 2025

@Slashek Thanks for your input, should be much better now.

render 'modules/common-styling/user/avatar', size: 's', name: name, imageSrc: other_participant.avatar.photo.versions.sm
render 'modules/common-styling/user/avatar', size: 's', name: name, imageSrc: from_profile.avatar.photo.versions.sm
endif
print name | raw_escape_string
Copy link
Contributor

Choose a reason for hiding this comment

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

can we just use {{ }} to ensure no XSS attacks etc? I'm not sure if raw_escape_string is enough

@Slashek
Copy link
Contributor

Slashek commented Jul 21, 2025

thx @lk316 , one last comment

@lk316
Copy link
Contributor Author

lk316 commented Jul 21, 2025

@Slashek fixed

@lk316 lk316 force-pushed the fix-conversation-title branch from 3aa5d31 to 24439f1 Compare August 6, 2025 16:36
@Slashek Slashek merged commit 0fd082c into main Sep 30, 2025
7 checks passed
@Slashek Slashek deleted the fix-conversation-title branch September 30, 2025 11:32
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