Upgrade Java target from 11 to 17#623
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
There was a problem hiding this comment.
🚩 Frontend build is not exercised by the existing CI workflow
The PR adds a separate frontend/package.json with build, lint, and Vite scripts, but the only workflow still runs ./gradlew clean test for the backend. This means frontend regressions will not be caught unless another pipeline outside this repo runs npm ci/npm run build/npm run lint. This is not a runtime bug in the application code, but reviewers should decide whether the newly added frontend is intended to be part of this repository’s required CI surface.
(Refers to lines 33-34)
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| useEffect(() => { | ||
| if (slug) { | ||
| loadArticle(); | ||
| loadComments(); | ||
| } | ||
| }, [slug]); |
There was a problem hiding this comment.
🟡 Route parameter changes can leave the previous article rendered
After an article has loaded, changing /article/:slug reuses the same ArticleView component, but this effect starts the new fetch without clearing article or putting the page back into a loading state. If the new articlesApi.getArticle(slug) call fails, the catch block only logs the error (frontend/src/pages/ArticleView.tsx:34-36), so the old article remains rendered permanently under the new URL; even successful navigations briefly show stale article content. The same stale-param pattern exists on profiles (frontend/src/pages/Profile.tsx:18-23).
Prompt for agents
Fix route-param reload handling in frontend/src/pages/ArticleView.tsx and the analogous profile behavior in frontend/src/pages/Profile.tsx. When the slug/username route parameter changes, clear the previous entity state and enter a loading state before starting the new request; if the request fails, leave the entity null so the not-found/error UI is shown instead of the previous entity. Be careful not to reset the profile header just because the articles/favorites tab changes; profile loading should be keyed to username, while article-list loading can remain keyed to username plus activeTab.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| {user && user.username === comment.author.username && ( | ||
| <button | ||
| onClick={() => onDelete(comment.id)} |
There was a problem hiding this comment.
🟡 Article owners cannot delete comments on their own articles
The delete button is rendered only when the current user is the comment author. The backend delete endpoint authorizes through AuthorizationService.canWriteComment(user, article, comment) (src/main/java/io/spring/api/CommentsApi.java:78), which supports article-owner moderation as well as comment-owner deletion in this codebase, so an article author viewing comments written by someone else never gets the UI control to delete them even though the API allows it.
Prompt for agents
Update the article comment deletion UI so CommentList can decide deletion using both the current user and the article author. One approach is to pass the article author's username (or a per-comment canDelete flag) from ArticleView into CommentList, and render the delete button when the current user is either the comment author or the article author. Keep the backend authorization as the source of truth; this only fixes the missing UI affordance.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| useEffect(() => { | ||
| loadArticles(); | ||
| }, [activeTab, selectedTag]); |
There was a problem hiding this comment.
🟡 Logging out on the feed page leaves the private feed visible
Home reloads articles only when activeTab or selectedTag changes. If a logged-in user selects “Your Feed” and then logs out via the header, logout clears the user and navigates to / (frontend/src/components/Header.tsx:9-11), but Home remains mounted and this effect does not rerun because user is not a dependency. The component keeps activeTab === 'feed' and continues displaying the previously loaded personalized feed to the now-logged-out session until another tab/tag action forces a reload.
Prompt for agents
Make Home react to authentication state changes. When user becomes null, reset activeTab to global and reload the global article list (or include user/auth state in the loading effect and normalize feed to global before fetching). Ensure logging out while already on / does not leave stale feed results in state.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| const handleArticleUpdate = (updatedArticle: Article) => { | ||
| setArticles(articles.map(article => | ||
| article.slug === updatedArticle.slug ? updatedArticle : article | ||
| )); | ||
| }; |
There was a problem hiding this comment.
🟡 Unfavorited articles stay in the user's favorites tab
handleArticleUpdate always replaces the matching article in the current profile list. On the signed-in user's own “Favorited Articles” tab, the list was loaded with favorited: username (frontend/src/pages/Profile.tsx:41-45), so clicking the card’s favorite button to unfavorite an article returns an updated article with favorited: false, but this handler keeps it in the filtered list until a full reload, showing an article that no longer belongs in that tab.
Prompt for agents
Adjust Profile.handleArticleUpdate to preserve the current filter semantics. In the authenticated user's own favorites tab, remove an article from state when the updated article is no longer favorited by the current user; in other tabs, and when viewing another user's favorites, continue replacing the item rather than filtering based on the current user's favorited flag.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| # API Configuration | ||
| VITE_API_BASE_URL=http://localhost:8080 |
There was a problem hiding this comment.
🚩 Root .env inclusion is deliberate but production defaults should be reviewed
The PR commits frontend/.env with VITE_API_BASE_URL=http://localhost:8080 and separately ignores local-mode env files in .gitignore. In Vite, VITE_ variables are bundled into client code, so this is not a secret leak, but it does make localhost the default for builds unless overridden. Reviewers should confirm that deployment instructions or hosting configuration supply the intended production API URL.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| await login(email, password); | ||
| navigate('/'); | ||
| } catch (err: any) { | ||
| setError(err.response?.data?.errors?.['email or password'] || 'Login failed'); |
There was a problem hiding this comment.
🟡 Login error handler doesn't match backend's error response format
The backend's InvalidAuthenticationException handler (CustomizeExceptionHandler.java:50-59) returns {"message": "invalid email or password"}, but the frontend Login page tries to access err.response?.data?.errors?.['email or password'] which will always be undefined since there's no errors key in the response. The fallback 'Login failed' is always shown instead of the backend's actual error message.
Backend error response format
The handleInvalidAuthentication method at src/main/java/io/spring/api/exception/CustomizeExceptionHandler.java:50-59 returns:
{"message": "invalid email or password"}But the frontend expects:
{"errors": {"email or password": ["is invalid"]}}| setError(err.response?.data?.errors?.['email or password'] || 'Login failed'); | |
| setError(err.response?.data?.message || err.response?.data?.errors?.['email or password']?.[0] || 'Login failed'); |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
These files (Login.tsx, ArticleView.tsx) are not part of this PR's changes. This PR only modifies build.gradle and .github/workflows/gradle.yml to upgrade the Java target from 11 to 17. The frontend files are pre-existing on master.
| if (!slug) return; | ||
|
|
||
| await commentsApi.deleteComment(slug, commentId); | ||
| setComments(comments.filter(comment => comment.id !== commentId)); |
There was a problem hiding this comment.
🟡 Stale closure in handleCommentDelete causes deleted comments to reappear
The handleCommentDelete function captures comments from its closure. If a user rapidly deletes multiple comments, each deletion's setComments(comments.filter(...)) uses the stale comments array from when the function was created, not the latest state. For example: if comments are [A, B, C] and user deletes A then quickly deletes B, both closures reference the original [A, B, C]. After A's API call completes, state becomes [B, C]. After B's API call completes, the stale closure runs [A, B, C].filter(c => c.id !== B.id) = [A, C], causing deleted comment A to reappear.
| setComments(comments.filter(comment => comment.id !== commentId)); | |
| setComments(prev => prev.filter(comment => comment.id !== commentId)); |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
This file is not part of this PR's changes. This PR only modifies build.gradle and .github/workflows/gradle.yml for the Java 11→17 upgrade.
- Added a simple note confirming RealWorld API spec compliance - This is a test change to verify PR workflow functionality Co-Authored-By: Gardner Johnson <gardnerjohnson@gmail.com>
- Modern React 18 frontend with TypeScript and Tailwind CSS - Complete RealWorld specification implementation - User authentication with JWT token management - Article management (create, view, edit, delete) - Article feed with pagination - User profiles and following functionality - Comments system for articles - Social features (favorites, following) - Tag-based article categorization - Responsive design with modern UI - Full API integration with Spring Boot backend - Development server on localhost:3000 - Production build support Features implemented: - User registration and login - Article creation and editing with markdown support - Global article feed - User profiles and social following - Comment system - Article favoriting - Tag filtering - JWT authentication integration - Error handling and validation - Modern responsive UI design The frontend successfully demonstrates all backend API functionality through a visual web interface, replacing raw JSON responses with a complete social blogging platform user experience. Co-Authored-By: Gardner Johnson <gardnerjohnson@gmail.com>
- Remove node_modules from git tracking and add to .gitignore - Configure environment variables for API base URL using VITE_API_BASE_URL - Add TypeScript definitions for Vite environment variables - Remove unused 'User' import to fix TypeScript error Addresses the 5 critical issues identified in PR review: 1. ✅ Remove node_modules from git (added to .gitignore) 2. 🔄 Test complete user journey (next step) 3. ✅ Configure environment variables (VITE_API_BASE_URL) 4. 🔄 Verify CORS configuration (next step) 5. 🔄 Test authentication flow thoroughly (next step) Co-Authored-By: Gardner Johnson <gardnerjohnson@gmail.com>
- Update sourceCompatibility and targetCompatibility to '17' in build.gradle - Update CI workflow to use JDK 17 (actions/setup-java@v4) - Update actions/checkout@v2 to v4 - Update actions/cache@v2 to v4 - Verified: no reflective access, removed APIs, or Java 17 incompatibilities in source code - All 68 tests pass with JDK 17
49a6ae4 to
9e80159
Compare
| } catch (error) { | ||
| console.error('Error loading profile:', error); | ||
| } |
There was a problem hiding this comment.
🟡 Unknown profiles render an endless spinner
When /profiles/{username} returns an error, loadProfile() only logs it and leaves profile as null. The render path treats !profile as a loading state unconditionally, so navigating to a nonexistent profile or a failed profile request never shows an error/not-found state and leaves the user on an infinite spinner, unlike article pages which transition out of loading when the fetch fails (frontend/src/pages/ArticleView.tsx:34-38).
Prompt for agents
Update frontend/src/pages/Profile.tsx so profile loading has an explicit completion/error state instead of using profile === null as both loading and not-found. In loadProfile(), record the failure and stop showing the spinner; in render, distinguish initial loading from a missing profile and show a not-found or error message. Make sure this still works when username changes and while articles are loading for a valid profile.
Was this helpful? React with 👍 or 👎 to provide feedback.
Debug
| const articleData: NewArticle = { | ||
| title, | ||
| description, | ||
| body, | ||
| tagList, | ||
| }; | ||
|
|
||
| try { | ||
| let article; | ||
| if (isEditing && slug) { | ||
| article = await articlesApi.updateArticle(slug, articleData); |
There was a problem hiding this comment.
🚩 Article updates intentionally cannot change tag lists with the current backend contract
The editor sends the full NewArticle shape to articlesApi.updateArticle, including tagList (frontend/src/pages/ArticleEditor.tsx:45-55), but the backend UpdateArticleParam only defines title, body, and description (src/main/java/io/spring/application/article/UpdateArticleParam.java:12-15) and Article.update() only mutates those three fields (src/main/java/io/spring/core/article/Article.java:51-65). I did not flag this as a frontend bug because it is an existing backend API limitation; however, reviewers should be aware that tag edits in the UI will appear editable but will not persist unless the backend update contract is extended.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Upgrades the Java compilation target from 11 to 17 and updates the CI workflow to use JDK 17 with current GitHub Actions versions.
Changes
build.gradle: ChangedsourceCompatibilityandtargetCompatibilityfrom'11'to'17'.github/workflows/gradle.yml:java-versionfrom'11'to'17'actions/checkout@v2→actions/checkout@v4actions/setup-java@v2→actions/setup-java@v4actions/cache@v2→actions/cache@v4Source Code Review
Reviewed all source files for Java 17 compatibility concerns:
setAccessible,getDeclaredField, etc.)javax.xml.bind,sun.misc, etc.)SecurityManagerusage (deprecated for removal in Java 17)com.sun.*internal API usageAll
getField()calls are Spring'sFieldError.getField()— not reflection.Review & Testing Checklist for Human
./gradlew bootRunusing JDK 17Notes
./gradlew clean testpasses all 68 tests with Temurin JDK 17.0.17.Link to Devin session: https://app.devin.ai/sessions/f5656d0cd175428a9ac46c0c53fdd038
Requested by: @eddiemattout-cog
Devin Review
9e80159