Conversation
There was a problem hiding this comment.
Hi @bestian, thank you for your latest contribution to Frankly -- this work is very helpful as always! 🙏 The PR mostly looks good after manual testing and works well. Most of these comments are just suggestions to remove extraneous comments.
There is just one main missing piece, which is that polls are not being downloaded in the polls-suggestions-data.csv files, despite the frontend indicating that polls will be included. This is because the frontend is all building off a backend function called GetMeetingChatSuggestionData, which is only returning chats and suggestions. We will need to modify that function and all downstream items to also return data for poll agenda items.
Let me know if you are interested in adding this piece, or if you would like me to collaborate on this! Thank you and hope you have a great weekend :)
client/lib/features/events/features/event_page/data/providers/event_provider.dart
Outdated
Show resolved
Hide resolved
client/lib/features/events/features/event_page/data/providers/event_provider.dart
Outdated
Show resolved
Hide resolved
client/lib/features/events/features/event_page/data/providers/event_provider.dart
Outdated
Show resolved
Hide resolved
client/lib/features/events/features/event_page/data/providers/event_provider.dart
Outdated
Show resolved
Hide resolved
client/lib/features/events/features/event_page/data/providers/event_provider.dart
Outdated
Show resolved
Hide resolved
client/lib/features/events/features/event_page/data/providers/event_provider.dart
Outdated
Show resolved
Hide resolved
client/lib/features/events/features/event_page/data/providers/event_provider.dart
Outdated
Show resolved
Hide resolved
client/lib/features/events/features/event_page/data/providers/event_provider.dart
Outdated
Show resolved
Hide resolved
client/lib/features/events/features/event_page/presentation/widgets/event_info.dart
Outdated
Show resolved
Hide resolved
...t/lib/features/events/features/event_page/presentation/widgets/event_pop_up_menu_button.dart
Outdated
Show resolved
Hide resolved
|
Thanks, Katherine! 🙏 Sorry for the missing piece in my original PR. It would be great if you could directly make the changes needed to include polls in the CSV, so the final result's function and format will match exactly what you have in mind. Thanks for collaborating, and wish you have a happy weekend, too.😊 |
…event_provider.dart Co-authored-by: Kathy Qian <[email protected]>
…event_provider.dart Co-authored-by: Kathy Qian <[email protected]>
…event_provider.dart Co-authored-by: Kathy Qian <[email protected]>
…dgets/event_info.dart Co-authored-by: Kathy Qian <[email protected]>
…event_provider.dart Co-authored-by: Kathy Qian <[email protected]>
…event_provider.dart Co-authored-by: Kathy Qian <[email protected]>
…event_provider.dart Co-authored-by: Kathy Qian <[email protected]>
…dgets/event_info.dart Co-authored-by: Kathy Qian <[email protected]>
…dgets/event_pop_up_menu_button.dart Co-authored-by: Kathy Qian <[email protected]>
…event_provider.dart Co-authored-by: Kathy Qian <[email protected]>
…event_provider.dart Co-authored-by: Kathy Qian <[email protected]>
…event_provider.dart Co-authored-by: Kathy Qian <[email protected]>
|
Hi~ @katherineqian , After further investigation, it looks like the issue happens during the data persistence stage. In testing, the poll data only appears while the meeting agenda is active — once the agenda ends, the poll data isn’t being saved. Even without reloading the page, as soon as we move to the next agenda and then click back to view the previous poll agenda, all options show zero results, which suggests the data was never stored.
I’m currently exploring possible fixes, but I wanted to document this root cause first so that you’re aware. If I don’t find a working solution on my end, at least this should point to the right direction for further debugging. I also mentioned this bug at isuue #245. Thanks! — Bestian |
|
Hi @katherineqian, Just a quick update after additional investigation — the issue still isn’t resolved, and at this point I’m not fully certain that it’s caused by missing writes during data persistence. There seem to be three possible areas to investigate further:
So far, I haven’t pinpointed the exact cause — just sharing these possible directions for further debugging. Unfortunately, I won’t be able to continue a deeper investigation in the near term, so I hope the core team can take it back from here. Sorry for not fully completing this feature update. |
|
Hi @bestian, thank you for your investigation and for providing these updates - super helpful to know you encountered this issue! I will keep you posted when this item is picked up again. 🙏 |
client/lib/features/events/features/event_page/data/providers/event_provider.dart
Show resolved
Hide resolved
mikewillems
left a comment
There was a problem hiding this comment.
Okay, fully reviewed. Great work overall!
See the couple comments about feedback de-anonymization.
Also, I actually had to review how flutter handles async to check the improved download handling. Awesome work there, but I read in this vein that there's a use_build_context_synchronously linter rule that we can enable in analysis_options.yaml to check for what you were fixing there. We should definitely enable that. Want to do that on this PR?
Lastly, only hard request: build regenerates these tracked files with changes - please go ahead and build locally then commit the changes:
modified: data_models/lib/events/live_meetings/live_meeting.g.dart
modified: data_models/lib/user_input/poll_data.freezed.dart
modified: data_models/lib/user_input/poll_data.g.dart
|
Thank you — I’ve rebuilt the project and resolved the merge conflicts. Sorry that the subsequent checks and updates go beyond my familiarity with the Flutter framework. My primary background is in Vue for frontend development and Cloudflare for backend work. Most of my contributions to Frankly have been made with assistance from AI. I want to avoid introducing any AI-generated inaccuracies that might create extra burden for human reviewers. For that reason, I plan to focus mainly on translation work moving forward. I’d appreciate it if you and the team could take over and complete this PR. Thank you very much for your support. |
|
@bestian Thank you for following up on this! Please feel no obligation to worry about this PR going forward :) To be clear, I will take responsibility for addressing the remaining comments and completing the collaborative effort on this feature. We all really appreciate all the work you put in on this PR! 🙌 |
|
Thanks for the review, @mikewillems! Seems like there weren't actually too many code changes requested -- as a summary:
|
There was a problem hiding this comment.
Pull request overview
This PR implements improved event data exports by splitting the previously combined export into three separate CSV files: Registration Data, Polls & Suggestions Data, and Chats Data. It also adds new fields requested in issue #182 and refactors the data_models/chat/ package contents into data_models/user_input/.
Changes:
- Added a new
GetMeetingPollDatacloud function andPollDatamodel to serve poll responses for export. - Refactored CSV generation in the event provider to produce three distinct export files with updated column structures (User ID, Join Time, Room Assigned, etc.), replacing the old single combined export.
- Relocated chat/emotion data models from
data_models/lib/chat/todata_models/lib/user_input/and updated all imports throughout the codebase.
Reviewed changes
Copilot reviewed 43 out of 51 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
firebase/functions/lib/events/live_meetings/get_meeting_poll_data.dart |
New cloud function that retrieves poll responses across main and breakout meeting rooms |
firebase/functions/node/main.dart |
Registers the new GetMeetingPollData function |
firebase/functions/test/events/live_meetings/get_meeting_poll_data_test.dart |
Tests for the new poll data function |
data_models/lib/user_input/poll_data.dart (+ .freezed.dart, .g.dart) |
New PollData model with freezed/JSON codegen |
data_models/lib/user_input/chat.dart, emotion.dart, chat_suggestion_data.dart (+ generated files) |
Chat/emotion models relocated from data_models/lib/chat/ |
data_models/lib/cloud_functions/requests.dart (+ generated files) |
New GetMeetingPollDataRequest/Response request/response types |
client/lib/features/events/features/event_page/data/providers/event_provider.dart |
Refactored CSV generation: new generateChatDataCsv, generatePollsSuggestionsDataCsv; updated generateRegistrationDataCsvFile |
client/lib/features/events/features/event_page/presentation/widgets/event_info.dart |
New _downloadChatData and _downloadPollsSuggestionsData methods; breakout room data fetching |
client/lib/features/events/features/event_page/presentation/widgets/event_pop_up_menu_button.dart |
Updated menu from 2 to 3 download buttons; switched from SVG to Material icons |
client/lib/features/events/features/event_page/presentation/event_page_presenter.dart |
Added getPollData() method |
client/lib/features/events/features/live_meeting/data/services/cloud_functions_live_meeting_service.dart |
Added getMeetingPollData() service method |
client/lib/l10n/app_en.arb, app_es.arb, app_zh.arb, app_zh_Hant_TW.arb |
Added new localization strings for the new download buttons |
.github/workflows/test_firebase.yaml |
Added JDK 21 setup step |
.github/workflows/function-mapping.json |
Added GetMeetingPollData to function mapping |
Various client/lib files importing data_models/chat/ |
Updated imports to new data_models/user_input/ path |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
client/lib/features/events/features/event_page/presentation/widgets/event_info.dart
Outdated
Show resolved
Hide resolved
client/lib/features/events/features/event_page/presentation/widgets/event_info.dart
Outdated
Show resolved
Hide resolved
firebase/functions/test/events/live_meetings/get_meeting_poll_data_test.dart
Show resolved
Hide resolved
client/lib/features/events/features/event_page/data/providers/event_provider.dart
Show resolved
Hide resolved
client/lib/features/events/features/event_page/data/providers/event_provider.dart
Outdated
Show resolved
Hide resolved
client/lib/features/events/features/event_page/data/providers/event_provider.dart
Show resolved
Hide resolved
client/lib/features/events/features/event_page/data/providers/event_provider.dart
Show resolved
Hide resolved
Update client/lib/features/events/features/event_page/data/providers/event_provider.dart Co-authored-by: Copilot <[email protected]>
… download functions (where it was repeated 3x).
…ead of trying to get it from the Firebase Auth record as was done in GetMeetingChatsSuggestionsData. Standardize the calls used in all 3 data download functions.
…injection attacks.
…st returning null in that case.

What is in this PR?
This PR implements the new export structure for event data, splitting the previous combined exports into three separate files:
The goal is to make the exported data clearer, better organized, and easier for admins to analyze.
I have completed the implementation and successfully tested all three export functions locally.
Changes in the codebase
Implemented three independent data export processes for Registration, Polls & Suggestions, and Chats.
Added new data fields as discussed in [issue #182](Event Data Improvements #182) (e.g.,
Join Time,Room Assigned,User ID).Refactored the old combined export logic to remove unnecessary fields and match the updated requirements.
Updated backend logic to handle new CSV structure and column names.
UI Note:
The export buttons in the interface have been updated from two buttons → three buttons.
Currently, the second and third buttons share the same icon because I did not design new icons.
Please update the button icons to whatever design is preferred.
Changes outside the codebase
Testing this PR
Go to the event management page and try each of the three export buttons.
Confirm that each CSV file downloads correctly and matches the expected structure:
Join TimeandRoom Assigned.Prompttext.If you encounter any missing or unclear details, please feel free to directly adjust the code for small refinements.
Additional information