SAK-44924 Calendar add search #13987
Conversation
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughAdds calendar search integration. Introduces a CalendarContentProducer for indexing calendar events, wires it via Spring, and adds required search dependencies. Updates search UI mappings and bundles to display calendar tool name and icon. No removals; new class, bean, properties, and minimal frontend adjustments. Changes
Sequence Diagram(s)sequenceDiagram
participant EventBus as Calendar Events
participant SearchService
participant CCP as CalendarContentProducer
participant CalSvc as CalendarService
participant SiteSvc as SiteService
participant Indexer as SearchIndexBuilder
EventBus->>SearchService: calendar.new / revise / delete
SearchService->>CCP: notify with reference
CCP->>CalSvc: get event by reference
CalSvc-->>CCP: CalendarEvent
CCP->>SiteSvc: verify site access (siteId)
SiteSvc-->>CCP: access result
alt add/update
CCP-->>Indexer: provide content, title, url
Indexer->>Indexer: index document
else delete
CCP-->>Indexer: remove document
end
sequenceDiagram
actor User
participant UI as SakaiSearch (frontend)
participant I18n as search.properties
User->>UI: enter query
UI->>UI: map tool "calendar" -> icon si-sakai-schedule
UI->>I18n: load toolname_calendar
I18n-->>UI: "Calendar"
UI-->>User: render results with icon and tool name
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
webcomponents/tool/src/main/frontend/packages/sakai-search/src/SakaiSearch.js (1)
22-33: Icon mapping for calendar looks correctKey matches producer getTool() = "calendar" and icon class looks right (si-sakai-schedule).
Consider a safe fallback to avoid "undefined" class if an unknown r.tool appears:
- this.iconMapping[r.tool] || "si si-sakai-help"
calendar/calendar-impl/impl/src/java/org/sakaiproject/calendar/impl/CalendarContentProducer.java (2)
340-347: Null-guard event in matches(Event) to avoid NPEsMinor safety improvement.
- public boolean matches(Event event) { - return matches(event.getResource()); - } + public boolean matches(Event event) { + return event != null && matches(event.getResource()); + }
339-342: Null-guard reference in matches(String)Minor safety improvement when called defensively.
- public boolean matches(String reference) { - return reference.startsWith(CalendarService.REFERENCE_ROOT); - } + public boolean matches(String reference) { + return reference != null && reference.startsWith(CalendarService.REFERENCE_ROOT); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
calendar/calendar-impl/impl/pom.xml(1 hunks)calendar/calendar-impl/impl/src/java/org/sakaiproject/calendar/impl/CalendarContentProducer.java(1 hunks)calendar/calendar-impl/impl/src/webapp/WEB-INF/components.xml(1 hunks)webcomponents/bundle/src/main/bundle/search.properties(1 hunks)webcomponents/tool/src/main/frontend/packages/sakai-search/src/SakaiSearch.js(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (3)
webcomponents/tool/src/main/frontend/packages/sakai-search/src/SakaiSearch.js (1)
37-49: Tool name mapping: ensure i18n key availability at runtimeMapping to this._i18n.toolname_calendar is correct. Please confirm the key exists in all locale bundles to avoid undefined labels in non-English UIs.
Use the script in the search.properties comment to verify locale coverage.
calendar/calendar-impl/impl/pom.xml (1)
184-191: DependencyManagement Verified for search-api and search-utilThe parent BOM (master/pom.xml lines 2063–2073) provides:
<dependencyManagement>entry for search-api with version${sakai.version}and scopeprovided<dependencyManagement>entry for search-util with version${sakai.version}(default compile scope)calendar/calendar-impl/impl/pom.xml inherits these, so no explicit
<version>or<scope>is required.
Please ensure your code importsorg.sakaiproject.search.util.SearchUtilsfrom the search-util artifact.calendar/calendar-impl/impl/src/java/org/sakaiproject/calendar/impl/CalendarContentProducer.java (1)
239-243: Verify getEvents(null, null) semantics and performanceConfirm this returns all events and is safe. Some Calendar API variants expect a TimeRange and/or type filters; nulls may not yield the intended result or may be costly.
Would you prefer we scope the range (e.g., past year + future year) and inject TimeService to build a TimeRange? I can draft that if desired.
| import org.sakaiproject.search.api.EntityContentProducer; | ||
| import org.sakaiproject.search.api.SearchIndexBuilder; | ||
| import org.sakaiproject.search.api.SearchService; | ||
| import org.sakaiproject.search.api.SearchUtils; |
There was a problem hiding this comment.
Wrong package for SearchUtils import (compile-time issue)
SearchUtils resides in search-util, typically under org.sakaiproject.search.util.SearchUtils, not org.sakaiproject.search.api.
Apply:
-import org.sakaiproject.search.api.SearchUtils;
+import org.sakaiproject.search.util.SearchUtils;📝 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.
| import org.sakaiproject.search.api.SearchUtils; | |
| import org.sakaiproject.search.util.SearchUtils; |
🤖 Prompt for AI Agents
In
calendar/calendar-impl/impl/src/java/org/sakaiproject/calendar/impl/CalendarContentProducer.java
around line 43, the file currently imports SearchUtils from the wrong package;
replace the incorrect import org.sakaiproject.search.api.SearchUtils with the
correct package org.sakaiproject.search.util.SearchUtils (remove the old import
and add the new one) so the class compiles against the search-util module.
| protected void init() throws Exception { | ||
| log.info("init()"); | ||
|
|
||
| if ("true".equals(serverConfigurationService.getString("search.enable", "false"))) { | ||
| for (Iterator<String> i = addEvents.iterator(); i.hasNext();) { | ||
| getSearchService().registerFunction((String) i.next()); | ||
| } | ||
| for (Iterator<String> i = removeEvents.iterator(); i.hasNext();) { | ||
| getSearchService().registerFunction((String) i.next()); | ||
| } | ||
| getSearchIndexBuilder().registerEntityContentProducer(this); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use boolean config and guard service nulls in init()
Avoid string comparisons and NPEs if services aren’t injected (defensive).
- if ("true".equals(serverConfigurationService.getString("search.enable", "false"))) {
+ if (serverConfigurationService != null
+ && serverConfigurationService.getBoolean("search.enable", false)
+ && searchService != null
+ && searchIndexBuilder != null) {
for (Iterator<String> i = addEvents.iterator(); i.hasNext();) {
getSearchService().registerFunction((String) i.next());
}
for (Iterator<String> i = removeEvents.iterator(); i.hasNext();) {
getSearchService().registerFunction((String) i.next());
}
getSearchIndexBuilder().registerEntityContentProducer(this);
}📝 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.
| protected void init() throws Exception { | |
| log.info("init()"); | |
| if ("true".equals(serverConfigurationService.getString("search.enable", "false"))) { | |
| for (Iterator<String> i = addEvents.iterator(); i.hasNext();) { | |
| getSearchService().registerFunction((String) i.next()); | |
| } | |
| for (Iterator<String> i = removeEvents.iterator(); i.hasNext();) { | |
| getSearchService().registerFunction((String) i.next()); | |
| } | |
| getSearchIndexBuilder().registerEntityContentProducer(this); | |
| } | |
| } | |
| protected void init() throws Exception { | |
| log.info("init()"); | |
| if (serverConfigurationService != null | |
| && serverConfigurationService.getBoolean("search.enable", false) | |
| && searchService != null | |
| && searchIndexBuilder != null) { | |
| for (Iterator<String> i = addEvents.iterator(); i.hasNext();) { | |
| getSearchService().registerFunction((String) i.next()); | |
| } | |
| for (Iterator<String> i = removeEvents.iterator(); i.hasNext();) { | |
| getSearchService().registerFunction((String) i.next()); | |
| } | |
| getSearchIndexBuilder().registerEntityContentProducer(this); | |
| } | |
| } |
| log.debug("Calendar Event Content for reference: {} is: {}", ref.getReference(), sb.toString()); | ||
| return sb.toString(); | ||
| } |
There was a problem hiding this comment.
Avoid logging full event content (PII leakage risk)
Debug log echoes event titles, descriptions, and locations into logs. Remove or redact to prevent PII leakage.
- log.debug("Calendar Event Content for reference: {} is: {}", ref.getReference(), sb.toString());
+ log.debug("Indexed calendar event content for reference: {}", ref.getReference());🤖 Prompt for AI Agents
In
calendar/calendar-impl/impl/src/java/org/sakaiproject/calendar/impl/CalendarContentProducer.java
around lines 207 to 209, the debug log currently emits the full event content
(title, description, location) which risks PII leakage; change the log to avoid
printing the full sb.toString() by either removing the content from the message
or redacting sensitive fields — e.g., log only the event reference/id and a
non-sensitive indicator such as content length or a hash, or replace
title/description/location with masked values; ensure any remaining debug output
contains no user-identifiable text.
| return "/portal/directtool/" + toolConfig.getId() + | ||
| "?eventReference=" + reference + | ||
| "&panel=Main&sakai_action=doDescription&sakai.state.reset=true"; | ||
| } | ||
| } catch (Exception e) { | ||
| log.debug("Error getting tool configuration for site: {}", siteId, e); | ||
| } | ||
|
|
||
| // Fallback to regular site URL | ||
| return "/portal/site/" + siteId + "/tool/sakai.schedule?eventReference=" + reference + | ||
| "&panel=Main&sakai_action=doDescription&sakai.state.reset=true"; | ||
| } |
There was a problem hiding this comment.
URL-encode query parameters to avoid broken links and injection issues
reference can contain reserved characters. Encode it for both directtool and site URLs.
- return "/portal/directtool/" + toolConfig.getId() +
- "?eventReference=" + reference +
+ String encodedRef = java.net.URLEncoder.encode(reference, java.nio.charset.StandardCharsets.UTF_8.name());
+ return "/portal/directtool/" + toolConfig.getId() +
+ "?eventReference=" + encodedRef +
"&panel=Main&sakai_action=doDescription&sakai.state.reset=true";
...
- return "/portal/site/" + siteId + "/tool/sakai.schedule?eventReference=" + reference +
+ String encodedRef = java.net.URLEncoder.encode(reference, java.nio.charset.StandardCharsets.UTF_8.name());
+ return "/portal/site/" + siteId + "/tool/sakai.schedule?eventReference=" + encodedRef +
"&panel=Main&sakai_action=doDescription&sakai.state.reset=true";Add imports if you prefer to pre-import:
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;🤖 Prompt for AI Agents
In
calendar/calendar-impl/impl/src/java/org/sakaiproject/calendar/impl/CalendarContentProducer.java
around lines 309 to 320, the reference query parameter is inserted raw into two
returned URLs which can break links or allow injection; URL-encode reference
before concatenation (e.g. use URLEncoder.encode(reference,
StandardCharsets.UTF_8.name())) for both the "/portal/directtool/..." and
"/portal/site/..." return paths, add imports for java.net.URLEncoder and
java.nio.charset.StandardCharsets, and handle the checked exception or let it
propagate appropriately (wrap encoding in a try/catch and log/throw if encoding
fails).
| <bean id="calendarSearchAdapter" | ||
| class="org.sakaiproject.calendar.impl.CalendarContentProducer" | ||
| init-method="init" destroy-method="destroy"> | ||
| <property name="addEvents"> | ||
| <list> | ||
| <value>calendar.new</value> | ||
| <value>calendar.revise</value> | ||
| <value>calendar.revise.event.title</value> | ||
| <value>calendar.revise.event.time</value> | ||
| <value>calendar.revise.event.type</value> | ||
| <value>calendar.revise.event.access</value> | ||
| <value>calendar.revise.event.frequency</value> | ||
| </list> | ||
| </property> | ||
| <property name="removeEvents"> | ||
| <list> | ||
| <value>calendar.delete</value> | ||
| <value>calendar.delete.event</value> | ||
| </list> | ||
| </property> | ||
| <property name="entityManager"> | ||
| <ref bean="org.sakaiproject.entity.api.EntityManager"/> | ||
| </property> | ||
| <property name="calendarService"> | ||
| <ref bean="org.sakaiproject.calendar.api.CalendarService"/> | ||
| </property> | ||
| <property name="searchService"> | ||
| <ref bean="org.sakaiproject.search.api.SearchService" /> | ||
| </property> | ||
| <property name="searchIndexBuilder"> | ||
| <ref bean="org.sakaiproject.search.api.SearchIndexBuilder" /> | ||
| </property> | ||
| <property name="siteService" ref="org.sakaiproject.site.api.SiteService"/> | ||
| <property name="serverConfigurationService" ref="org.sakaiproject.component.api.ServerConfigurationService"/> | ||
| </bean> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Event trigger coverage: consider adding description/location revise events
To keep the index in sync, please verify whether description and location changes emit distinct events (e.g., calendar.revise.event.description, calendar.revise.event.location). If they exist, add them to addEvents.
Use this to search for available calendar event names:
Optionally, add an explicit depends-on to ensure search services are initialized before registering:
<bean id="calendarSearchAdapter"
class="org.sakaiproject.calendar.impl.CalendarContentProducer"
init-method="init" destroy-method="destroy"
depends-on="org.sakaiproject.search.api.SearchService, org.sakaiproject.search.api.SearchIndexBuilder">
...
</bean>🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for calendar.* event keys..."
rg -n 'calendar\.(new|revise|delete)[\w\.]*' calendar | sort -uLength of output: 18194
Review calendar event coverage and update adapter
No dedicated events exist for description or location changes (the API emits only the generic calendar.revise for those). However, two additional event keys—calendar.revise.event.exclusions and calendar.revise.event.excluded—are defined in CalendarService but not wired in the adapter. To ensure index consistency, add them to the addEvents list.
• File: calendar/calendar-impl/impl/src/webapp/WEB-INF/components.xml (lines ~92–126)
Update the <list> under <property name="addEvents">:
<value>calendar.revise.event.frequency</value>
+ <value>calendar.revise.event.exclusions</value>
+ <value>calendar.revise.event.excluded</value>Optionally, you may still include a depends-on on the search beans for init ordering:
<bean id="calendarSearchAdapter"
class="org.sakaiproject.calendar.impl.CalendarContentProducer"
init-method="init" destroy-method="destroy"
depends-on="org.sakaiproject.search.api.SearchService,org.sakaiproject.search.api.SearchIndexBuilder">
…
</bean>🤖 Prompt for AI Agents
In calendar/calendar-impl/impl/src/webapp/WEB-INF/components.xml around lines 92
to 126, the CalendarContentProducer bean's addEvents list is missing two event
keys (calendar.revise.event.exclusions and calendar.revise.event.excluded)
defined in CalendarService; add these two <value> entries into the <property
name="addEvents"> list so the adapter listens for those changes and keeps the
search index consistent, and optionally add a
depends-on="org.sakaiproject.search.api.SearchService,org.sakaiproject.search.api.SearchIndexBuilder"
attribute to the <bean id="calendarSearchAdapter" ...> to ensure init ordering
with search beans.
| @@ -19,3 +19,4 @@ toolname_forum=Forum | |||
| toolname_lesson=Lesson | |||
| toolname_resources=Resources | |||
| toolname_wiki=Wiki | |||
| toolname_calendar=Calendar | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add locale coverage for toolname_calendar
This addition looks good. Please ensure the same key exists across the other search.properties locale bundles; otherwise non-English UIs will show a missing label.
Run to verify presence across all search*.properties bundles:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Scanning for toolname_calendar across search.*.properties files..."
fd -a -t f 'search.*\.properties' | while read -r f; do
if rg -n '^\s*toolname_calendar=' "$f" > /dev/null; then
echo "OK: $f"
else
echo "MISSING: $f"
fi
doneLength of output: 3560
Add toolname_calendar entry to all localized search bundles
Scan results show that only the default search.properties contains the new key. To avoid missing UI labels in non-English locales, please add toolname_calendar (with the appropriate translated value) to each of the following files:
• webcomponents/bundle/src/main/bundle/search_ar.properties
• webcomponents/bundle/src/main/bundle/search_bg.properties
• webcomponents/bundle/src/main/bundle/search_ca.properties
• webcomponents/bundle/src/main/bundle/search_de_DE.properties
• webcomponents/bundle/src/main/bundle/search_es.properties
• webcomponents/bundle/src/main/bundle/search_eu.properties
• webcomponents/bundle/src/main/bundle/search_fr_FR.properties
• webcomponents/bundle/src/main/bundle/search_ro_RO.properties
• webcomponents/bundle/src/main/bundle/search_sr.properties
• webcomponents/bundle/src/main/bundle/search_tr_TR.properties
Once added, ensure each translation is reviewed for accuracy.
🤖 Prompt for AI Agents
In webcomponents/bundle/src/main/bundle/search.properties around line 22 and in
all localized bundles listed (search_ar.properties, search_bg.properties,
search_ca.properties, search_de_DE.properties, search_es.properties,
search_eu.properties, search_fr_FR.properties, search_ro_RO.properties,
search_sr.properties, search_tr_TR.properties), the new key toolname_calendar is
missing; add a toolname_calendar entry to each localized file with the
appropriate translated value for "Calendar" (matching locale conventions), place
it alongside other toolname_* keys, and have each translation reviewed for
accuracy by a native speaker or translator before committing.
TODO:
icon mapping
attachment url to calendar
Summary by CodeRabbit