feat: add intro dialog #3571
feat: add intro dialog #3571
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an intro dialog feature to display release information (v2.17) to users. The feature tracks whether users have seen the intro for the current version using a per-user preference stored in the backend.
Changes:
- Added IntroDialog.vue component with a carousel for displaying release content
- Integrated intro dialog into App.vue to show on first login after version update
- Added backend API endpoints to track and persist whether users have seen the intro
- Added binary asset (poster.webp) for intro content
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/components/IntroDialog.vue | New component for intro dialog with carousel (currently empty) |
| web/src/App.vue | Integration of intro dialog, shown based on seen_intro flag |
| api/system_info.go | Added logic to check/set seen_intro preference per user |
| api/router.go | Added POST endpoint for marking intro as seen |
| web/src/assets/intro/poster.webp | Binary asset for intro content |
| // Check if the user has seen the intro | ||
| seenIntroKey := fmt.Sprintf("seen_intro_%d", user.ID) | ||
| seenIntroVersion, err := helpers.Store(r).GetOption(seenIntroKey) | ||
| seenIntro := seenIntroVersion == util.Ver |
There was a problem hiding this comment.
The 'seenIntro' variable is calculated before checking if 'GetOption' returned an error. If an error occurs, 'seenIntroVersion' may be an empty string or other unexpected value, leading to incorrect comparison with 'util.Ver'. The calculation should happen after the error check, or a default value should be used on error.
| <v-carousel> | ||
| <v-carousel-item> | ||
|
|
||
| </v-carousel-item> | ||
| <v-carousel-item> </v-carousel-item> |
There was a problem hiding this comment.
The carousel component is empty with no content. Users will see a blank dialog when the intro is displayed. The carousel items should contain actual content like release notes, feature highlights, or other introductory information.
| <v-carousel> | |
| <v-carousel-item> | |
| </v-carousel-item> | |
| <v-carousel-item> </v-carousel-item> | |
| <v-carousel | |
| cycle | |
| hide-delimiter-background | |
| > | |
| <v-carousel-item> | |
| <v-container fluid> | |
| <v-row> | |
| <v-col cols="12"> | |
| <h2 class="text-h5 mb-4">What's new in v2.17</h2> | |
| </v-col> | |
| <v-col cols="12"> | |
| <ul class="mb-2"> | |
| <li>Improved job log viewer for large outputs.</li> | |
| <li>Faster project dashboard loading times.</li> | |
| <li>Enhanced secrets management and permissions.</li> | |
| </ul> | |
| <p class="mt-2 mb-0"> | |
| Explore these improvements in your existing projects, or create a new project to try them out. | |
| </p> | |
| </v-col> | |
| </v-row> | |
| </v-container> | |
| </v-carousel-item> | |
| <v-carousel-item> | |
| <v-container fluid> | |
| <v-row> | |
| <v-col cols="12"> | |
| <h2 class="text-h5 mb-4">Getting started & tips</h2> | |
| </v-col> | |
| <v-col cols="12"> | |
| <ul class="mb-2"> | |
| <li>Review your project settings to configure notifications and access control.</li> | |
| <li>Use templates to standardize common jobs across your team.</li> | |
| <li>Check the documentation for detailed upgrade notes for v2.17.</li> | |
| </ul> | |
| <p class="mt-2 mb-0"> | |
| You can reopen this dialog from the help menu if you want to review these notes later. | |
| </p> | |
| </v-col> | |
| </v-row> | |
| </v-container> | |
| </v-carousel-item> |
| this.item = null; | ||
| this.template = null; | ||
| this.dialog = val; | ||
| await this.loadData(); |
There was a problem hiding this comment.
The 'loadData' method is called but never implemented. This will attempt to call an undefined method when the dialog value changes, potentially causing runtime errors.
| projectId: Number, | ||
| itemId: Number, | ||
| systemInfo: Object, |
There was a problem hiding this comment.
The component accepts 'projectId', 'itemId', 'template', and 'systemInfo' props that are never used. These props appear to be copied from a template or another component and should be removed to avoid confusion.
| projectId: Number, | |
| itemId: Number, | |
| systemInfo: Object, |
| this.item = null; | ||
| this.template = null; |
There was a problem hiding this comment.
The watcher sets 'this.item' and 'this.template' to null, but these properties don't exist in the component's data object. This suggests incomplete refactoring from a template component.
| data() { | ||
| return { | ||
| dialog: null, | ||
| model: 0, |
There was a problem hiding this comment.
The 'model' data property is defined but never used in the component. This should be removed to keep the code clean.
| model: 0, |
| onClose() { | ||
| this.$emit('close'); | ||
| }, |
There was a problem hiding this comment.
There's no API call to persist that the user has seen the intro dialog. When the dialog is closed, the backend should be notified via the '/info/seen_intro' endpoint so the dialog doesn't show again on the next login.
| if err != nil { | ||
| log.WithError(err).Error("Failed to get seen_intro option") | ||
| http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) | ||
| return | ||
| } |
There was a problem hiding this comment.
The error handling returns an HTTP error but doesn't set 'seenIntro' to a default value. This means the response body construction on line 91-103 will fail due to an undefined variable. Either return early (which is already done) or set a default value before the error check.
No description provided.