Skip to content

fix: actually create a talk room when converting proposal #7398

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
Merged
SebastianKrupinski merged 1 commit into from
Sep 25, 2025
Merged

fix: actually create a talk room when converting proposal #7398

SebastianKrupinski merged 1 commit into from
Sep 25, 2025

Conversation

@SebastianKrupinski
Copy link
Contributor

@SebastianKrupinski SebastianKrupinski commented Sep 11, 2025 •
edited
Loading

Summary

  • Creates a talk room when a proposal is converted to a event.

Testing

  • Create meeting proposal, with add talk conversation selected
  • Convert a meeting proposal date to a calendar appointment/event
  • Then open the appointment and click on the talk link

@SebastianKrupinski SebastianKrupinski self-assigned this Sep 11, 2025
@SebastianKrupinski SebastianKrupinski added the 3. to review Waiting for reviews label Sep 11, 2025
@github-project-automation github-project-automation bot moved this to 🏗️ In progress in 💌 📅 👥 Groupware team Sep 11, 2025
@codecov
Copy link

codecov bot commented Sep 11, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 13.57%. Comparing base (f189e36) to head (2ed846e).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #7398   +/-   ##
=======================================
  Coverage   13.56%   13.57%           
=======================================
  Files         218      218           
  Lines       11329    11321    -8     
  Branches     2710     2711    +1     
=======================================
  Hits         1537     1537           
+ Misses       9432     9424    -8     
  Partials      360      360
Flag Coverage Δ
javascript 13.57% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@SebastianKrupinski SebastianKrupinski marked this pull request as draft September 11, 2025 01:39
@SebastianKrupinski SebastianKrupinski added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Sep 11, 2025
@SebastianKrupinski SebastianKrupinski force-pushed the fix/meeting-proposals-talk-room branch 2 times, most recently from 06cc391 to b476d3d Compare September 11, 2025 21:39
@SebastianKrupinski SebastianKrupinski marked this pull request as ready for review September 11, 2025 21:42
@SebastianKrupinski SebastianKrupinski force-pushed the fix/meeting-proposals-talk-room branch from b476d3d to 97be1e6 Compare September 11, 2025 21:46
st3iny
st3iny requested changes Sep 12, 2025
View reviewed changes
Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

Breaks when Talk is disabled.

You need to add a check if Talk is not installed/enabled and hide the Add Talk conversation checkbox.

@SebastianKrupinski
Copy link
Contributor Author

SebastianKrupinski commented Sep 12, 2025

Breaks when Talk is disabled.

You need to add a check if Talk is not installed/enabled and hide the Add Talk conversation checkbox.

Ah yes, that is a donkey oversight on my part.

@SebastianKrupinski
Copy link
Contributor Author

SebastianKrupinski commented Sep 12, 2025

Breaks when Talk is disabled.

You need to add a check if Talk is not installed/enabled and hide the Add Talk conversation checkbox.

Done. Added two checks

  • Check one in the editor to hide the the talk conversation option
  • Check two in the store in case the proposal was created with the option selected but talk is turned off for some reason later
  • Also removed the redundant location input lock as the location field is now hidden when the "add talk conversation" is selected

st3iny
st3iny reviewed Sep 17, 2025
View reviewed changes
src/services/talkService.ts Outdated
Comment on lines 68 to 82
if (participant.realm === ProposalParticipantRealm.Internal) {
// Try resolving internal participant to a user via autocomplete service
try {
const matches: AutocompleteEntry[] = await autocomplete({ search: participant.address, limit: 1 })
if (matches[0] !== undefined && matches[0].shareWithDisplayNameUnique === participant.address) {
users.push(matches[0].id)
continue
}
// Fallback to email if no good user match
emails.push(participant.address)
} catch {
// On errors, fallback to email
emails.push(participant.address)
}
} else {
Copy link
Member

@st3iny st3iny Sep 17, 2025

Choose a reason for hiding this comment

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

This does not seem to work. On my instance, a user is always added via email and thus not invited to the room.

Copy link
Contributor Author

@SebastianKrupinski SebastianKrupinski Sep 24, 2025

Choose a reason for hiding this comment

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

Hey,

So I finally had a look at this, but I am not sure, why your room is getting created with emails only,

It is discovering the users properly
image

And seems to be creating the room with users proproperly
image

Copy link
Contributor Author

@SebastianKrupinski SebastianKrupinski Sep 24, 2025

Choose a reason for hiding this comment

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

I had another thought on this, I think maybe the issue was that you had contacts with the same email address as the user and the contacts where being returned first.

Also made the whole process async to make it faster and fixed a bug in the backend

Copy link
Member

@st3iny st3iny Sep 25, 2025

Choose a reason for hiding this comment

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

Indeed, I clicked on the SAB contact instead of the actual user. Perhaps the SAB can be skipped when searching for contacts? Users are shown anyway.

(Not required for this PR.)

st3iny
st3iny reviewed Sep 17, 2025
View reviewed changes
Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

Tested again but found another bug.

@SebastianKrupinski SebastianKrupinski mentioned this pull request Sep 18, 2025
Closed
6 tasks
@SebastianKrupinski SebastianKrupinski force-pushed the fix/meeting-proposals-talk-room branch from d38e871 to e788de3 Compare September 24, 2025 13:30
@SebastianKrupinski
fix: actually create a talk room when converting proposal
2ed846e 
Signed-off-by: SebastianKrupinski <krupinskis05@gmail.com>
@SebastianKrupinski SebastianKrupinski force-pushed the fix/meeting-proposals-talk-room branch from 8bf2740 to 2ed846e Compare September 25, 2025 11:55
@SebastianKrupinski SebastianKrupinski merged commit 6cda00b into main Sep 25, 2025
41 checks passed
@SebastianKrupinski SebastianKrupinski deleted the fix/meeting-proposals-talk-room branch September 25, 2025 12:06
@github-project-automation github-project-automation bot moved this from 🏗️ In progress to ☑️ Done in 💌 📅 👥 Groupware team Sep 25, 2025
@SebastianKrupinski
Copy link
Contributor Author

SebastianKrupinski commented Sep 25, 2025

/backport to stable6.0

@backportbot backportbot bot added the backport-request A backport was requested for this pull request label Sep 25, 2025
@backportbot backportbot bot mentioned this pull request Sep 25, 2025
Merged
2 tasks
@backportbot backportbot bot removed the backport-request A backport was requested for this pull request label Sep 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@st3iny st3iny st3iny approved these changes

@tcitworld tcitworld Awaiting requested review from tcitworld tcitworld is a code owner

Assignees

@SebastianKrupinski SebastianKrupinski

Labels

2. developing Work in progress

Projects

💌 📅 👥 Groupware team
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@SebastianKrupinski @st3iny