[BRE-1281] Enhance ios release notes formatting

[brandonbiete]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/BRE-1281

📔 Objective

Enhance iOS Release Notes Formatting

📸 Screenshots

⏰ Reminders before review

• Contributor guidelines followed
• All formatters and local linters executed and passed
• Written new unit and / or integration tests where applicable
• Protected functional changes with optionality (feature flags)
• Used internationalization (i18n) for all UI strings
• CI builds passed
• Communicated to DevOps any deployment requirements
• Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

• 👍 (:+1:) or similar for great changes
• 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
• ❓ (:question:) for questions
• 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
• 🎨 (:art:) for suggestions / improvements
• ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
• 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
• ⛏ (:pick:) for minor or nitpick changes

[claude]

Claude finished @brandonbiete's task —— View job

---

Code Review Complete

Tasks:

• Invoke reviewing-changes skill
• Analyze changed files in detail
• Identify critical issues
• Suggest improvements
• Note good practices
• Provide comprehensive review feedback

---

Summary of Changes

This PR enhances the iOS release notes formatting for the App Store submission workflow. The changes span two files:

1. GitHub Actions Workflow (.github/workflows/publish-store.yml): Modified to support dry-run mode properly and pass the dry-run flag to Fastlane
2. Fastfile (fastlane/Fastfile): Enhanced the release_to_production lane to:
   • Format changelog text by splitting on periods and adding bullet points
   • Strip existing bullet points from input
   • Add dry-run mode that validates release notes without submitting to App Store Connect
   • Provide clear UI feedback distinguishing dry-run vs production modes

---

Critical Issues

⚠️ 1. Changelog Formatting Logic May Break Multi-Sentence Items

Location: fastlane/Fastfile:458-463

The current implementation splits changelog on every period (.), which will incorrectly break multi-sentence changelog items:

formatted_changelog = changelog
  .split('.')  # ← Splits on ALL periods
  .map(&:strip)
  .reject(&:empty?)
  .map { |item| "• #{item.gsub(/\A(?:•|\u2022)\s*/, '')}" }
  .join("\n")

Problem Examples:

• Input: "Fixed bug in v2.0. Added new feature"
• Expected: 2 bullet points
• Input: "Updated U.S.A. localization. Fixed error"
• Actual: Creates bullet for "U" and "S" and "A"

Impact: High - This will produce malformed release notes for any changelog containing periods within items (abbreviations, version numbers, decimal numbers, etc.)

Recommendation:

• Use a more robust delimiter (e.g., newlines, double periods, or explicit list markers)
• Document the expected input format for changelog parameter
• Add validation/warning if periods are detected in unexpected places

---

⚠️ 2. Nil Changelog Handling May Mask Configuration Issues

Location: fastlane/Fastfile:455

changelog = options[:changelog] || "Bug fixes and improvements"

The workflow provides CHANGELOG: ${{ inputs.changelog }} which can be an empty string (not nil). This means if a user doesn't provide a changelog, they may get an empty string that passes through formatting, resulting in no release notes or malformed output.

Impact: Medium - Could result in empty or default release notes being published without operator awareness

Recommendation:

changelog = options[:changelog]
if changelog.nil? || changelog.strip.empty?
  UI.user_error!("Changelog is required for App Store submission")
end

Or if defaults are acceptable, make it explicit:

changelog = options[:changelog]
if changelog.nil? || changelog.strip.empty?
  UI.important("⚠️  No changelog provided, using default")
  changelog = "Bug fixes and improvements"
end

---

⚠️ 3. Regex Pattern for Bullet Point Removal May Miss Unicode Variants

Location: fastlane/Fastfile:462

.map { |item| "• #{item.gsub(/\A(?:•|\u2022)\s*/, '')}" }

The pattern only matches literal • and \u2022 (which are the same character). It misses:

• Other bullet point characters: ▪, ▫, ◦, ‣, ⁃, -, *
• Numbered lists: 1., 2.
• Other list markers

Impact: Low-Medium - Existing bullets may not be stripped, resulting in double bullets like • • Item

Recommendation:

.map { |item| "• #{item.gsub(/\A(?:•|▪|▫|◦|‣|⁃|\*|-|\d+\.)\s*/, '')}" }

---

Suggested Improvements

📝 1. Inconsistent Conditional Check in Workflow

Location: .github/workflows/publish-store.yml:158

if ${{ inputs.dry-run }} ; then

This uses a different style than the shell variable check above it (if [ "$DRY_RUN" = "true" ]).

Recommendation: Use consistent checking:

if [ "$DRY_RUN" = "true" ]; then

---

📝 2. Dry Run Exit Doesn't Stop Subsequent Steps

Location: fastlane/Fastfile:503

The next keyword exits the lane but returns success, which means:

• The "Enable Publish Github Release Workflow" step will run even in dry-run mode
• Line 158 in the workflow catches this, but it's redundant protection

Recommendation: Consider whether the workflow step condition is sufficient or if the Fastlane lane should explicitly signal dry-run completion differently.

---

📝 3. Missing Input Validation

Location: fastlane/Fastfile:453-522

The lane doesn't validate:

• build_number is provided and valid
• app_version is provided and valid
• metadata_path exists and is accessible
• app_identifier is one of the expected values

Recommendation: Add validation at the start of the lane:

required_options = [:app_identifier, :build_number, :app_version, :metadata_path]
ensure_required_options(options, required_options)

---

📝 4. Changelog Formatting Should Be Tested

Location: fastlane/Fastfile:458-463

This formatting logic has multiple edge cases but no apparent test coverage.

Recommendation: Add unit tests or at minimum, add comments documenting:

• Expected input format
• Example inputs and outputs
• Known limitations

---

📝 5. Dry Run Validation Is Superficial

Location: fastlane/Fastfile:496-503

The dry-run mode shows success messages but doesn't actually validate:

• Credentials are valid
• Build number exists in TestFlight
• App version matches expectations
• Metadata path is correct

Recommendation: Consider adding validation steps in dry-run mode:

if is_dry_run
  UI.important("🧪 DRY RUN MODE - Validating configuration...")
  
  # Validate metadata path exists
  unless Dir.exist?("./#{metadata_path}")
    UI.user_error!("Metadata path does not exist: #{metadata_path}")
  end
  
  # Could add API calls to verify build exists, etc.
  
  UI.message("✅ Release notes prepared for #{locales.count} locales")
  # ... rest of output
end

---

Good Practices Observed

✅ Uses boolean coercion for is_dry_run to handle both string and boolean inputs
✅ Provides clear UI feedback with emojis distinguishing dry-run vs production
✅ Maintains backward compatibility with default changelog value
✅ Creates locale-specific release note files properly
✅ Uses early exit pattern in dry-run mode to prevent accidental submissions

---

Action Items for Author

1. Critical: Fix changelog splitting logic to handle periods within items (e.g., abbreviations, version numbers)
2. Critical: Decide on and document the expected input format for changelog parameter
3. High: Add explicit validation for empty/nil changelog values
4. Medium: Expand bullet point stripping regex to cover common variants
5. Medium: Add input validation for required options in release_to_production lane
6. Low: Make conditional checks consistent between bash and GitHub Actions syntax
7. Consider: Add actual validation in dry-run mode beyond just formatting

---

📋 Example Test Cases for Changelog Formatting

The formatting logic should handle these cases:

# Expected behavior:
"Feature A. Feature B" 
# → "• Feature A\n• Feature B"

"Feature A. Feature B. Feature C"
# → "• Feature A\n• Feature B\n• Feature C"

# Edge cases that may break:
"Updated U.S.A. localization. Fixed bug"
# Currently: "• Updated U\n• S\n• A\n• localization\n• Fixed bug"
# Expected: "• Updated U.S.A. localization\n• Fixed bug"

"Fixed issue in v2.0.1. Added feature"
# Currently: "• Fixed issue in v2\n• 0\n• 1\n• Added feature"
# Expected: "• Fixed issue in v2.0.1\n• Added feature"

"• Already bulleted. • Another item"
# Should strip existing bullets and format correctly

Consider requiring a different delimiter or explicit line breaks between items.

---

[github-actions]

Checkmarx One – Scan Summary & Details – acc323e3-4e97-457b-8fdb-0afdf75692dd

Great job! No new security vulnerabilities introduced in this pull request

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.68%. Comparing base (1f5b9fc) to head (85c3752).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2103      +/-   ##
==========================================
- Coverage   85.24%   83.68%   -1.57%     
==========================================
  Files        1709     1979     +270     
  Lines      145484   161298   +15814     
==========================================
+ Hits       124020   134980   +10960     
- Misses      21464    26318    +4854     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.