Conversation
resolves #356
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2181 +/- ##
============================================
- Coverage 15.19% 15.18% -0.02%
Complexity 391 391
============================================
Files 232 232
Lines 6204 6209 +5
Branches 717 717
============================================
Hits 943 943
- Misses 5210 5215 +5
Partials 51 51 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Caution Review failedThe pull request is closed. ## Walkthrough
A new non-nullable `repoName` field was added to the `Application` entity and its database schema. The application creation and edit forms now include this field, and UI components display a GitHub repository link based on it. The dispatcher servlet sets a default value for `repoName` during database population. The `ApplicationVersion.getFileUrl()` method was updated to use `repoName` instead of the package name for GitHub URL construction. Temporary APK storage was reorganized into language-specific subdirectories. Discord notification code was removed.
## Changes
| Files/Paths | Change Summary |
|---------------------------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------|
| `src/main/java/ai/elimu/entity/application/Application.java` | Added `repoName` field with `@NotNull` annotation and documentation. |
| `src/main/resources/META-INF/jpa-schema-export.sql` | Added `repoName` column to `Application` table. |
| `src/main/java/ai/elimu/web/servlet/CustomDispatcherServlet.java` | Set `repoName` property to `"sound-cards"` in `populateDatabase` method. |
| `src/main/webapp/WEB-INF/jsp/application/create.jsp`,<br>`edit.jsp` | Added "Repo name" input field to creation/edit forms; replaced static package name display with disabled input; added GitHub repo link in edit form aside. |
| `src/main/webapp/WEB-INF/jsp/application-version/create.jsp` | Added "GitHub repo" link section to "Resources" aside panel using `repoName`. |
| `src/main/java/ai/elimu/entity/application/ApplicationVersion.java` | Modified `getFileUrl()` to use `repoName` instead of derived package name for GitHub URL path segment. |
| `src/main/java/ai/elimu/rest/v2/applications/ApplicationVersionsRestController.java` | Removed Discord notification; changed APK temp storage to language-specific subdirectory. |
| `src/main/java/ai/elimu/web/application/application_version/ApplicationVersionCreateController.java` | Changed APK temp storage to language-specific subdirectory; added logging for directory creation. |
| `pom-dependency-tree.txt` | Updated main artifact version from `2.6.3-SNAPSHOT` to `2.6.5-SNAPSHOT`. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant WebApp
participant Database
User->>WebApp: Open Application create/edit page
WebApp->>Database: Fetch Application (if editing)
WebApp-->>User: Display form with 'Repo name' field
User->>WebApp: Submit form with repoName
WebApp->>Database: Save Application with repoName
User->>WebApp: View Application version page
WebApp->>Database: Fetch ApplicationVersion and Application
WebApp-->>User: Display GitHub repo link using repoName
Note over WebApp: APK files saved in language-specific temp directoryAssessment against linked issues
Possibly related PRs
Suggested reviewers
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/webapp/WEB-INF/jsp/application/create.jsp (1)
18-21: Field added with appropriate styling and placeholderThe new repository name field has been correctly added to the form with proper label, error handling classes, and a helpful placeholder example. The column layout maintains UI consistency with the package name field.
Consider adding a hint about the expected format for the repository name, especially if it's intended to be used in constructing GitHub URLs.
<div class="col s12 m6 input-field"> <form:label path="repoName" cssErrorClass="error">Repo name</form:label> <form:input path="repoName" cssErrorClass="error" placeholder="soundcards" /> + <div class="helper-text">GitHub repository name (without organization name)</div> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
pom-dependency-tree.txt(1 hunks)src/main/java/ai/elimu/entity/application/Application.java(1 hunks)src/main/java/ai/elimu/web/servlet/CustomDispatcherServlet.java(1 hunks)src/main/resources/META-INF/jpa-schema-export.sql(1 hunks)src/main/webapp/WEB-INF/jsp/application/application-version/create.jsp(1 hunks)src/main/webapp/WEB-INF/jsp/application/create.jsp(1 hunks)src/main/webapp/WEB-INF/jsp/application/edit.jsp(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: test_ui
- GitHub Check: build (windows-latest, 17)
- GitHub Check: build (windows-latest, 21)
- GitHub Check: build (macos-latest, 21)
- GitHub Check: build (macos-latest, 17)
- GitHub Check: build (ubuntu-latest, 21)
- GitHub Check: build (ubuntu-latest, 17)
- GitHub Check: test_rest
- GitHub Check: test_rest
- GitHub Check: test_rest
- GitHub Check: test_rest
- GitHub Check: test_rest
🔇 Additional comments (6)
pom-dependency-tree.txt (1)
1-1: Version bump to support new featuresThe artifact version has been updated from 2.6.3-SNAPSHOT to 2.6.5-SNAPSHOT, which aligns with the new
repoNamefield implementation.src/main/java/ai/elimu/web/servlet/CustomDispatcherServlet.java (1)
293-293: Default value for repoName correctly setThe new
repoNamefield is properly initialized with "soundcards" during database population, which aligns with thepackageNamevalue format.src/main/resources/META-INF/jpa-schema-export.sql (1)
111-111:Details
❌ Incorrect review comment
Schema updated for new field
The
repoNamecolumn has been correctly added to the Application table schema. The column type (varchar(255)) is appropriate for storing repository names.However, consider whether this column should have a NOT NULL constraint to match the non-nullable nature of the field in the entity class.
🏁 Script executed:
#!/bin/bash # Check if the Application entity has a non-nullable repoName field fd -e java "Application.java" --exec grep -A 10 "repoName" {} \;Length of output: 374
Approved: Schema Addition of repoName
TherepoName varchar(255)column has been correctly added to theApplicationtable and aligns with the JPA default (nullable) setting on theprivate String repoName;field. No NOT NULL constraint is required unless you intend to enforce non-nullability at the application level.• If you do want to make
repoNamenon-nullable, annotate the field inApplication.javawith@Column(nullable = false)(or add a Bean Validation@NotNull), then update the DDL to:- repoName varchar(255), + repoName varchar(255) NOT NULL,• Otherwise, no further changes are needed.
Likely an incorrect or invalid review comment.
src/main/java/ai/elimu/entity/application/Application.java (1)
27-31: Well-documented addition of the repoName fieldThe addition of the non-nullable
repoNamefield with clear documentation explaining its purpose is good. This field will help maintain a consistent way to reference GitHub repositories throughout the application.src/main/webapp/WEB-INF/jsp/application/edit.jsp (2)
19-26: Good implementation of form fields for package and repo nameThe changes to display the package name as a disabled field while adding an editable field for the repo name makes sense. This allows users to edit the repository name while preserving the immutability of the package name.
169-179: Well-structured resources section with GitHub repository linkThe addition of the "Resources" section with a link to the GitHub repository is well-implemented. The link is properly constructed and opens in a new tab, allowing users to easily access the repository while maintaining their current context.
Issue Number
fileUrlto ApplicationVersion #2153Purpose
Technical Details
Testing Instructions
Screenshots
Format Checks
Note
Files in PRs are automatically checked for format violations with
mvn spotless:check.If this PR contains files with format violations, run
mvn spotless:applyto fix them.