-
Notifications
You must be signed in to change notification settings - Fork 81
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
Added the ability to generate Eco News from the Manage Eco News Page #8083
Added the ability to generate Eco News from the Manage Eco News Page #8083
Conversation
WalkthroughThe pull request introduces an AI-generated Eco News feature for the GreenCity application. The changes span multiple files to implement a new functionality that allows users to generate news articles using AI. The implementation includes modifications to the controller, JavaScript interactions, HTML templates, and localization files to support a seamless user experience for generating AI-powered Eco News content. Changes
Sequence DiagramsequenceDiagram
participant User
participant Browser
participant AIController
participant AIService
User->>Browser: Click "Generate AI Eco News"
Browser->>AIController: Send generation request
AIController->>AIService: Request news generation
AIService-->>AIController: Return generated news
AIController-->>Browser: Send generated content
Browser->>User: Display generated news in form
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (2)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (6)
service/src/main/java/greencity/service/EcoNewsServiceImpl.java (2)
104-107
: Consider extracting rating and achievement calculations into a separate method.The combination of rating calculation, achievement calculation, and notification creation appears in multiple places. Consider extracting these operations into a reusable method to improve maintainability and ensure consistent behavior.
Example refactor:
+ private void processUserAchievementsAndRatings(UserVO userVO, EcoNews news) { + achievementCalculation.calculateAchievement( + userVO, + AchievementCategoryType.CREATE_NEWS, + AchievementAction.ASSIGN + ); + ratingCalculation.ratingCalculation( + ratingPointsRepo.findByNameOrThrow("CREATE_NEWS"), + userVO + ); + userNotificationService.createNewNotification( + userVO, + NotificationType.ECONEWS_CREATED, + news.getId(), + news.getTitle() + ); + }This would simplify the calling code to:
- achievementCalculation.calculateAchievement(userVO, AchievementCategoryType.CREATE_NEWS, AchievementAction.ASSIGN); - ratingCalculation.ratingCalculation(ratingPointsRepo.findByNameOrThrow("CREATE_NEWS"), userVO); - userNotificationService.createNewNotification(userVO, NotificationType.ECONEWS_CREATED, toSave.getId(), toSave.getTitle()); + processUserAchievementsAndRatings(userVO, toSave);
Line range hint
1-1000
: Review transaction boundaries for rating calculations.The service uses class-level
@Transactional
, but consider defining explicit transaction boundaries for atomic operations involving rating calculations, achievements, and notifications to ensure data consistency.Consider:
- Using method-level
@Transactional(propagation = Propagation.REQUIRED)
for atomic operations- Adding explicit error handling for rating calculation failures
- Defining rollback behavior for specific exceptions
core/src/main/java/greencity/controller/AIController.java (1)
63-64
: Consider improving locale handling robustness.While the special case for Ukrainian locale works, consider these improvements:
- Add null check for locale
- Consider using a locale mapping configuration instead of hardcoding "українська"
- aiService.getNews(locale.toString().equals("ua") ? "українська" : locale.getDisplayLanguage(), query)); + aiService.getNews( + Optional.ofNullable(locale) + .map(l -> l.toString().equals("ua") ? "українська" : l.getDisplayLanguage()) + .orElse("english"), + query));core/src/main/resources/static/management/econews/buttonsAJAX.js (2)
349-353
: Consider using jQuery for consistency.Since jQuery is already used throughout the file, consider using it for DOM manipulation here as well.
- const form = document.getElementById('generateEcoNewsForm'); - form.style.display = form.style.display === 'none' ? 'block' : 'none'; + $('#generateEcoNewsForm').toggle();
355-387
: Add loading state and input validation.Consider these improvements for better user experience:
- Add loading state during content generation
- Validate query input before making the request
- Disable the generate button during the request
$('#generateEcoNewsContent').on('click', function (event) { const query = $('#generateQueryInput').val().trim(); + if (!query) { + $('#errorModalGenerateContent').text('Please enter a topic for generation'); + return; + } const language = localStorage.getItem("language") || "en"; const locale = language === "ua" ? "uk-UA" : "en-US"; + const $generateBtn = $(this); + $generateBtn.prop('disabled', true); + $('#generateSpinner').show(); $.ajax({ // ... existing ajax config ... success: function (response) { // ... existing success handling ... + $generateBtn.prop('disabled', false); + $('#generateSpinner').hide(); }, error: function (xhr, status, error) { // ... existing error handling ... + $generateBtn.prop('disabled', false); + $('#generateSpinner').hide(); } }); });core/src/main/resources/templates/core/management_eco_news.html (1)
535-542
: Enhance accessibility and user experience.Consider the following improvements:
- Add ARIA attributes for better screen reader support
- Add loading state handling for the generate button
Apply these changes:
- <input type="text" id="generateQueryInput" class="form-control"> + <input type="text" id="generateQueryInput" class="form-control" + aria-label="AI generation topic input"> - <span th:id="errorModalGenerateContent" id="errorModalGenerateContent" class="errorSpan"></span> + <span th:id="errorModalGenerateContent" id="errorModalGenerateContent" + class="errorSpan" role="alert" aria-live="polite"></span> - <button type="button" class="btn btn-secondary" id="generateEcoNewsContent"> + <button type="button" class="btn btn-secondary" id="generateEcoNewsContent" + data-loading-text="Generating...">
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
core/src/main/java/greencity/controller/AIController.java
(1 hunks)core/src/main/resources/messages.properties
(2 hunks)core/src/main/resources/messages_ua.properties
(2 hunks)core/src/main/resources/static/management/econews/buttonsAJAX.js
(1 hunks)core/src/main/resources/templates/core/management_eco_news.html
(1 hunks)service/src/main/java/greencity/service/EcoNewsServiceImpl.java
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (5)
service/src/main/java/greencity/service/EcoNewsServiceImpl.java (1)
105-105
: Great optimization in rating calculation!The direct use of
userVO
instead of mapping fromtoSave
simplifies the code and improves performance by eliminating unnecessary object mapping.core/src/main/resources/messages.properties (1)
181-181
: LGTM! Clear and consistent messages.The new localization entries follow the existing naming pattern and provide clear user instructions.
Also applies to: 210-211
core/src/main/resources/messages_ua.properties (1)
166-166
: LGTM! Proper Ukrainian translations.The Ukrainian translations are properly encoded and maintain consistency with the English messages.
Also applies to: 199-200
core/src/main/resources/templates/core/management_eco_news.html (2)
532-534
: Well-structured toggle button implementation!The button follows the application's styling conventions and properly implements internationalization.
532-543
: Clean integration with existing modal structure!The AI generation form is well-integrated into the existing modal, maintaining consistency with the application's design patterns and user experience.
console.log(response); | ||
const titleMatch = response.match(/\*\*Title:\s*(.+?)\s*\*\*/) || response.match(/\*\*\s*(.+?)\s*\*\*/); |
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.
🛠️ Refactor suggestion
Remove console.log of potentially sensitive data.
The response from the AI service might contain sensitive information. Remove the console.log or replace it with appropriate logging.
- console.log(response);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
console.log(response); | |
const titleMatch = response.match(/\*\*Title:\s*(.+?)\s*\*\*/) || response.match(/\*\*\s*(.+?)\s*\*\*/); | |
const titleMatch = response.match(/\*\*Title:\s*(.+?)\s*\*\*/) || response.match(/\*\*\s*(.+?)\s*\*\*/); |
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
core/src/main/resources/static/management/econews/buttonsAJAX.js (1)
349-353
: Consider using jQuery for consistent DOM manipulation.Since the codebase predominantly uses jQuery, let's maintain consistency:
$('#toggleGenerateEcoNewsForm').on('click', function (event) { - const form = document.getElementById('generateEcoNewsForm'); - form.style.display = form.style.display === 'none' ? 'block' : 'none'; + $('#generateEcoNewsForm').toggle(); });
$('#generateEcoNewsContent').on('click', function (event) { | ||
const query = $('#generateQueryInput').val().trim(); | ||
const language = localStorage.getItem("language") || "en"; | ||
const locale = language === "ua" ? "uk-UA" : "en-US"; | ||
|
||
$.ajax({ | ||
url: '/ai/generate/eco-news', | ||
type: 'GET', | ||
data: { query: query }, | ||
headers: { 'Accept-Language': locale }, | ||
contentType: 'application/json', | ||
success: function (response) { | ||
const titleMatch = response.match(/\*\*Title:\s*(.+?)\s*\*\*/) || response.match(/\*\*\s*(.+?)\s*\*\*/); | ||
|
||
if (titleMatch) { | ||
const title = titleMatch[1]; | ||
$('#inputTitle').val(title); | ||
|
||
const updatedContent = response.replace(titleMatch[0], '').trim(); | ||
|
||
tinymce.get('ecoNewsContent').setContent(updatedContent); | ||
} else { | ||
tinymce.get('ecoNewsContent').setContent(response); | ||
} | ||
}, | ||
error: function (xhr, status, error) { | ||
console.error('Error generating Eco News content:', status, error); | ||
document.getElementById("errorModalGenerateContent").innerText = | ||
'Failed to generate Eco News content. Please try again.'; | ||
} | ||
}); | ||
}); |
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.
🛠️ Refactor suggestion
Add input validation and improve user experience.
The implementation needs several improvements for robustness and user experience:
- Validate input before making the API call
- Show loading state during API call
- Remove console.error in production
- Add debounce to prevent rapid-fire API calls
$('#generateEcoNewsContent').on('click', function (event) {
const query = $('#generateQueryInput').val().trim();
+ if (!query) {
+ document.getElementById("errorModalGenerateContent").innerText =
+ 'Please enter a topic for content generation.';
+ return;
+ }
+
+ const $button = $(this);
+ $button.prop('disabled', true);
+ document.getElementById("errorModalGenerateContent").innerText = 'Generating content...';
+
const language = localStorage.getItem("language") || "en";
const locale = language === "ua" ? "uk-UA" : "en-US";
$.ajax({
url: '/ai/generate/eco-news',
type: 'GET',
data: { query: query },
headers: { 'Accept-Language': locale },
contentType: 'application/json',
success: function (response) {
const titleMatch = response.match(/\*\*Title:\s*(.+?)\s*\*\*/) || response.match(/\*\*\s*(.+?)\s*\*\*/);
if (titleMatch) {
const title = titleMatch[1];
$('#inputTitle').val(title);
const updatedContent = response.replace(titleMatch[0], '').trim();
tinymce.get('ecoNewsContent').setContent(updatedContent);
} else {
tinymce.get('ecoNewsContent').setContent(response);
}
+ document.getElementById("errorModalGenerateContent").innerText = '';
},
error: function (xhr, status, error) {
- console.error('Error generating Eco News content:', status, error);
+ // Log to server-side error tracking
document.getElementById("errorModalGenerateContent").innerText =
'Failed to generate Eco News content. Please try again.';
},
+ complete: function() {
+ $button.prop('disabled', false);
+ }
});
});
Also, consider adding debounce to prevent rapid-fire API calls:
const debouncedGenerate = _.debounce(function(event) {
// Move the entire click handler logic here
}, 1000);
$('#generateEcoNewsContent').on('click', debouncedGenerate);
|
GreenCity PR
Issue Link 📋
#8050
Summary Of Changes 🔥
Added the ability to generate eco news from the Manage Eco News Page.
Fixed an issue with rating calculation when creating eco news.
Added
Changed
PR Checklist Forms
(to be filled out by PR submitter)
dev
branch.NEED_REVIEW
andREADY_FOR_REVIEW
labels are added.Summary by CodeRabbit
New Features
Localization
User Interface