-
Notifications
You must be signed in to change notification settings - Fork 132
feature: added hidden rooms and ensure sync #1184
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
base: enext
Are you sure you want to change the base?
feature: added hidden rooms and ensure sync #1184
Conversation
Reviewer's GuideAdds support for unscheduled (hidden) rooms and a setup workflow by introducing new room visibility and state fields, filtering hidden rooms in backend queries and serializers, and updating admin and user-facing Vue components to manage and display room setup status and visibility. Sequence diagram for room setup completion and sidebar visibility syncsequenceDiagram
actor Admin
participant EditFormVue
participant API
participant Backend
Admin->>EditFormVue: Select room type and complete setup
EditFormVue->>API: PATCH room.config with setup_complete, hidden, sidebar_hidden
API->>Backend: save_room(...)
Backend->>Backend: If module_config and not setup_complete, set setup_complete True, sidebar_hidden False
Backend->>API: Return updated room config
API->>EditFormVue: Update room state
EditFormVue->>SidebarVue: Room now visible if setup_complete and not hidden/sidebar_hidden
Entity relationship diagram for updated Room modelerDiagram
ROOM {
INTEGER id
STRING name
STRING description
INTEGER pretalx_id
JSON schedule_data
BOOLEAN force_join
BOOLEAN setup_complete
BOOLEAN hidden
BOOLEAN sidebar_hidden
JSON module_config
}
ROOM ||--o{ AVAILABILITY : has
ROOM ||--o{ TALK_SLOT : has
TALK_SLOT {
INTEGER id
DATETIME start
DATETIME end
INTEGER room
INTEGER schedule
STRING description
}
AVAILABILITY {
INTEGER id
INTEGER room
STRING details
}
Class diagram for updated Room and related classesclassDiagram
class Room {
+int id
+str name
+str description
+int pretalx_id
+dict schedule_data
+bool force_join
+bool setup_complete
+bool hidden
+bool sidebar_hidden
+list module_config
}
class TalkSlot {
+int id
+datetime start
+datetime end
+Room room
+Schedule schedule
+str description
}
class Availability {
+int id
+Room room
...
}
Room "1" --o "*" TalkSlot : has
Room "1" --o "*" Availability : has
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- The hard-coded "Unscheduled Rooms" header should be wrapped in a translation key (using $t) to support localization.
- There’s a lot of repetition of
query: $route.queryin your router-link definitions—consider abstracting that into a computed property or mixin to DRY up the code. - Unscheduled rooms are rendered in insertion order; consider applying the same sorting (e.g. by sorting_priority or name) as the other room categories for consistency.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The hard-coded "Unscheduled Rooms" header should be wrapped in a translation key (using $t) to support localization.
- There’s a lot of repetition of `query: $route.query` in your router-link definitions—consider abstracting that into a computed property or mixin to DRY up the code.
- Unscheduled rooms are rendered in insertion order; consider applying the same sorting (e.g. by sorting_priority or name) as the other room categories for consistency.
## Individual Comments
### Comment 1
<location> `app/eventyay/base/models/room.py:204-207` </location>
<code_context>
pretalx_id = models.IntegerField(default=0)
schedule_data = JSONField(null=True, blank=True)
force_join = models.BooleanField(default=False)
+ is_unscheduled = models.BooleanField(
+ default=False,
+ help_text=_('Unscheduled rooms are video-only rooms not linked to any Talk session (e.g., networking lounges, help desks)')
+ )
</code_context>
<issue_to_address>
**suggestion (performance):** is_unscheduled field added to Room model.
If you anticipate frequent filtering by is_unscheduled, consider adding a database index to optimize query performance.
```suggestion
is_unscheduled = models.BooleanField(
default=False,
db_index=True,
help_text=_('Unscheduled rooms are video-only rooms not linked to any Talk session (e.g., networking lounges, help desks)')
)
```
</issue_to_address>
### Comment 2
<location> `app/eventyay/base/models/schedule.py:605` </location>
<code_context>
rooms = set() if not all_rooms else set(self.event.rooms.filter(is_unscheduled=False))
</code_context>
<issue_to_address>
**suggestion (code-quality):** Swap if/else branches of if expression to remove negation ([`swap-if-expression`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/swap-if-expression))
```suggestion
rooms = set(self.event.rooms.filter(is_unscheduled=False)) if all_rooms else set()
```
<br/><details><summary>Explanation</summary>Negated conditions are more difficult to read than positive ones, so it is best
to avoid them where we can. By swapping the `if` and `else` conditions around we
can invert the condition and make it positive.
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for "unscheduled" rooms, which are video-only rooms not linked to any Talk session (e.g., networking lounges, help desks). These rooms exist independently of the schedule and are excluded from schedule-related views and queries.
Key changes:
- Adds
is_unscheduledboolean field to the Room model with database migration - Implements UI for creating unscheduled rooms via a separate button and query parameter
- Filters unscheduled rooms from schedule-related queries and room listings
- Displays unscheduled rooms in a dedicated section in the sidebar
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| app/eventyay/base/models/room.py | Adds is_unscheduled boolean field to Room model with help text |
| app/eventyay/base/migrations/0002_room_is_unscheduled.py | Database migration to add the new is_unscheduled field |
| app/eventyay/api/serializers/rooms.py | Includes is_unscheduled field in room serializer |
| app/eventyay/base/services/event.py | Includes is_unscheduled in room config response |
| app/eventyay/webapp/src/views/admin/rooms/index.vue | Adds "Create Unscheduled Room" button with secondary styling |
| app/eventyay/webapp/src/views/admin/rooms/new.vue | Propagates unscheduled query parameter and sets initial config values |
| app/eventyay/webapp/src/views/admin/rooms/EditForm.vue | Sends is_unscheduled field when saving room configuration |
| app/eventyay/webapp/src/components/RoomsSidebar.vue | Displays unscheduled rooms in a dedicated section |
| app/eventyay/webapp/src/views/admin/rooms/RoomListItem.vue | Adds ellipsis styling to room names |
| app/eventyay/orga/views/schedule.py | Filters out unscheduled rooms from schedule views |
| app/eventyay/base/models/schedule.py | Excludes unscheduled rooms from talk schedule data |
Comments suppressed due to low confidence (1)
app/eventyay/webapp/src/views/admin/rooms/EditForm.vue:10
- The
pretalx_idinput field is only shown for scheduled rooms ('stage' or 'channel-bbb' types), but unscheduled rooms can be of any type. Consider hiding or disabling this field whenconfig.is_unscheduledis true, as unscheduled rooms should always havepretalx_idset to 0.
bunt-input(v-if="inferredType.id === 'stage' || inferredType.id === 'channel-bbb'", name="pretalx_id", v-model="config.pretalx_id", label="pretalx ID", :validation="v$.config.pretalx_id")
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f3821fc to
6755553
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- There’s a lot of duplicated filtering logic for
visibleRoomsacross App.vue, RoomsSidebar.vue, and other components—consider extracting this into a shared computed/helper to keep behavior consistent and reduce duplication. - The watchers for
config.hidden,config.sidebar_hidden, andconfig.setup_completecontain overlapping side effects that are hard to follow—think about consolidating this into a single reactive flow (e.g. using computed setters or a dedicated method) to simplify state transitions. - You switched the reorder API call to send an array of string IDs—please verify the backend endpoint expects strings and not integers to avoid potential type mismatches.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There’s a lot of duplicated filtering logic for `visibleRooms` across App.vue, RoomsSidebar.vue, and other components—consider extracting this into a shared computed/helper to keep behavior consistent and reduce duplication.
- The watchers for `config.hidden`, `config.sidebar_hidden`, and `config.setup_complete` contain overlapping side effects that are hard to follow—think about consolidating this into a single reactive flow (e.g. using computed setters or a dedicated method) to simplify state transitions.
- You switched the reorder API call to send an array of string IDs—please verify the backend endpoint expects strings and not integers to avoid potential type mismatches.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
7a0a8b8 to
9e504a7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 14 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
6c99318 to
b4dcc18
Compare
b4dcc18 to
4ca2e01
Compare
|
rebased and resolved conflicts |
| @@ -0,0 +1,61 @@ | |||
| from django.db import migrations, models | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a pending migration AFAIR, shouldn't we do that before? Is it fine to ignore that one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That migration needs to run before (or along with) the deployment, otherwise the new room fields and visibility logic won’t work correctly.
Fixes #1078



Summary by Sourcery
Add visibility and setup workflow for rooms by introducing new fields, admin UI for type selection and hiding rooms, and update backend and frontend to filter out hidden or incomplete rooms from scheduling and navigation.
New Features:
Bug Fixes:
Enhancements: