SAK-44773 SignUp change default value for attendance tracking

[tsarchghs]

this seems ok? it has additional check so it's working as expected I get the tab in the excel after enabling attendance

Summary by CodeRabbit

• Changes
  • Attendance tracking is now enabled by default for signup events; attendance export is available in signup worksheets.
• Configuration
  • signup.enableAttendance setting now defaults to true; can be toggled to disable attendance features.
  • signup.maxAttendeesPerSlot documented with a default value of 50.
• Documentation
  • Descriptions updated to clarify attendance feature and export behavior.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

EventWorksheet.isAttendanceOn(...) now treats the signup.enableAttendance configuration as enabled by default (true). The properties bundle documentation was updated for signup.enableAttendance and a signup.maxAttendeesPerSlot entry was added/adjusted in the default properties file.

Changes

Cohort / File(s)	Change Summary
Event logic signup/tool/src/main/java/org/sakaiproject/signup/tool/downloadEvents/EventWorksheet.java	isAttendanceOn(List<SignupMeetingWrapper> wrappers) now reads signup.enableAttendance with a default of true (previously false); method still returns true if any wrapper’s meeting allows attendance, otherwise false.
Configuration properties config/configuration/bundles/src/bundle/org/sakaiproject/config/bundle/default.sakai.properties	Updated signup.enableAttendance documentation to indicate default TRUE and show example assignment; added/modified signup.maxAttendeesPerSlot block/value and surrounding formatting.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: updating the default value for attendance tracking in SignUp, which is reflected in both the Java code and configuration changes.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 293d67c and 1c03f18.

📒 Files selected for processing (1)

• config/configuration/bundles/src/bundle/org/sakaiproject/config/bundle/default.sakai.properties

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: ottenhoff
Repo: sakaiproject/sakai PR: 0
File: :0-0
Timestamp: 2025-10-07T15:11:27.298Z
Learning: In samigo’s Total Scores view (samigo/samigo-app/src/webapp/jsf/evaluation/totalScores.jsp), mailto links were hidden after commit dee05746 (PR #12312, SAK-49674) added a render check requiring email.fromEmailAddress to be non-empty; PR #14154 (SAK-52058) restores visibility by checking only description.email.

🔇 Additional comments (1)

config/configuration/bundles/src/bundle/org/sakaiproject/config/bundle/default.sakai.properties (1)

4911-4915: signup.maxAttendeesPerSlot block looks consistent

The default/sample pattern here (DEFAULT: 500, example override 50) matches conventions used elsewhere and clearly communicates that 500 is the built‑in default and 50 is just a sample override. No issues from my side.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[tsarchghs]

I think I didn't check that if (wrp.getMeeting().isAllowAttendance()){ is just a wrapper for excel I thought it was checking the meeting's setting, waiting for some guidance on how to fix this.

thanks in advance.

[ern]

also need to update the config setting in default.sakai.properties

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 50c1c84 and c7f78b1.

📒 Files selected for processing (1)

• config/configuration/bundles/src/bundle/org/sakaiproject/config/bundle/default.sakai.properties

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: ottenhoff
Repo: sakaiproject/sakai PR: 0
File: :0-0
Timestamp: 2025-10-07T15:11:27.298Z
Learning: In samigo’s Total Scores view (samigo/samigo-app/src/webapp/jsf/evaluation/totalScores.jsp), mailto links were hidden after commit dee05746 (PR #12312, SAK-49674) added a render check requiring email.fromEmailAddress to be non-empty; PR #14154 (SAK-52058) restores visibility by checking only description.email.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Consolidate signup.enableAttendance docs and fix conflicting sample values

signup.enableAttendance is documented three times here, with conflicting sample values (true in one block, false in two others) and overlapping descriptions. This is confusing for deployers and makes it unclear which block to follow.

I suggest:

• Keep a single, clear block for signup.enableAttendance.
• Describe both attendance tracking and the Excel export sheet in that block.
• Keep DEFAULT: TRUE in the comment, and use a non-default sample value (false) to follow the pattern used elsewhere in this file (sample shows how to override the default).
• Remove the duplicate blocks.

Proposed cleanup for the attendance property docs

@@
-# signup.enableAttendance
-# enable the attendance feature and attendance export sheet in Sign Up
-# DEFAULT: TRUE
-# signup.enableAttendance=true
-
-
@@
-# signup.enableAttendance
-# if true, it provide the ability to take attendence of the event
-# DEFAULT: TRUE
-# signup.enableAttendance=false
-
-# signup.maxAttendeesPerSlot
+# signup.maxAttendeesPerSlot
@@
-# Signup attendance
-# if true, it provide the ability to take attendence of the event
-# DEFAULT: TRUE
-# signup.enableAttendance=false
-
-# Signup other sites availability
+# Signup other sites availability

And replace the first block (now removed) with a single, clarified one:

+# signup.enableAttendance
+# If true, enables attendance tracking for Sign Up meetings and adds the attendance sheet to the Excel export.
+# DEFAULT: TRUE
+# signup.enableAttendance=false
+

Also applies to: 4911-4915, 4952-4955

🤖 Prompt for AI Agents

In
config/configuration/bundles/src/bundle/org/sakaiproject/config/bundle/default.sakai.properties
around lines 4873-4877 (and likewise at 4911-4915 and 4952-4955), there are
three duplicate docs for signup.enableAttendance with conflicting sample values;
consolidate them into a single comment block that documents both attendance
tracking and the Excel export sheet, retain "DEFAULT: TRUE" and change the
sample override to "signup.enableAttendance=false" (a non-default example), and
remove the other two duplicate blocks so only the clarified single block
remains.

[jesusmmp]

If the default is true, the commented value is always the opposite :)