GSA / GSA/touchpoints

Eliminate silent save failures throughout codebase

Open
#2,048 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
67
Forks
37
Avg merge
2d 12h
Merged PRs (30d)
5

Description

'Silent save failure' refers to situations where the application fails to persist a change to the database but does nothing to clean up after the failure or to notify the user that the change was not persisted. Frequently, in such cases, the UI tells the user that the change _was_ saved and the user only learns about the failure when they reload the page or try to use the unsaved entity in some other context. This makes for a confusing user experience.

I asked an AI to scan the Touchpoints codebase for places where save failures are not handled. I'm pasting part of its report below. I myself encountered issue number 1, the `Form#duplicate!` problem.

----

### 1. **Form#duplicate! - Lines 236-249 (form.rb)**
**Severity:** HIGH

Multiple `.save` calls without error handling during form duplication:
```ruby
new_form_section.save # Line 236 - NO ERROR HANDLING
new_question.save # Line 243 - NO ERROR HANDLING
new_question_option.save # Line 249 - NO ERROR HANDLING
```

**Risk:** If any of these saves fail (validation errors, database issues), the duplicate form will be incomplete and corrupted without any warning to the user.

**Impact:**
- Orphaned form sections without questions
- Orphaned questions without options
- Incomplete copied forms

---

### 2. **CxCollection#duplicate! - Line 79 (cx_collection.rb)**
**Severity:** HIGH

```ruby
new_collection.save # NO ERROR HANDLING
```

**Risk:** Similar to Form#duplicate!, CX collection duplication can fail silently, creating incomplete copies.

---
### 3. **Admin FormsController#update_display_logo - Lines 132-139 (admin/forms_controller.rb)**
**Severity:** MEDIUM

```ruby
def update_display_logo
@form.update({...}) # Line 133 - NO ERROR HANDLING
# ... more code ...
@form.update(form_logo_params) # Line 138 - NO ERROR HANDLING
end
```

**Risk:**
- Multiple update calls without checking success
- No error messages sent to UI if updates fail
- User believes form is updated when it might not be

---
### 5. **Admin FormsController#update_notification_emails - Line 148 (admin/forms_controller.rb)**
**Severity:** MEDIUM

```ruby
def update_notification_emails
notification_emails = params[:emails]
@form.update_attribute(:notification_emails, notification_emails) # NO ERROR HANDLING
render json: @form
end
```

**Risk:**
- `update_attribute` bypasses validations
- Fails silently if save operation fails
- No error feedback to the API consumer

**Note:** `update_attribute` is deprecated and doesn't raise errors on failure.

---

### 6. **Admin FormsController#update_admin_options - Line 153 (admin/forms_controller.rb)**
**Severity:** MEDIUM

```ruby
def update_admin_options
@form.update(form_admin_options_params) # NO ERROR HANDLING
flash.now[:notice] = 'Admin form options updated successfully'
end
```

**Risk:**
- Flash message always shown regardless of save success
- User sees success message even if update failed
- No error handling path

---

### 7. **User.from_omniauth - Line 62 (user.rb)**
**Severity:** MEDIUM

```ruby
@existing_user.save # NO ERROR HANDLING
```

**Risk:**
- Authentication flow may not complete if save fails
- No user feedback about the failure
- Potential security/audit trail impact

---

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.