diff --git a/Gemfile.lock b/Gemfile.lock index 4a8207c..35c435a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -84,7 +84,7 @@ GEM bindex (0.8.1) bootsnap (1.22.0) msgpack (~> 1.2) - brakeman (8.0.2) + brakeman (8.0.4) racc builder (3.3.0) bundler-audit (0.9.3) diff --git a/PLAN.md b/PLAN.md index 531172e..d37cf0f 100755 --- a/PLAN.md +++ b/PLAN.md @@ -254,21 +254,21 @@ bundle exec rails runner "puts 'OK'" **Unit Tests** -- [ ] Registration#cancelable? - applied 상태 + 마감 전만 true -- [ ] Registration#cancel! - 멱등성: 이미 취소면 성공 반환 -- [ ] 마감 후 cancel! → NotCancelableError +- [x] Registration#cancelable? - applied 상태 + 마감 전만 true +- [x] Registration#cancel! - 멱등성: 이미 취소면 성공 반환 +- [x] 마감 후 cancel! → NotCancelableError **Integration Tests** -- [ ] confirmation_code + 이름으로 조회 성공 -- [ ] 잘못된 정보로 조회 → 에러 메시지 -- [ ] 취소 성공 → status 변경, canceled_at 기록 -- [ ] 이미 취소된 신청 다시 취소 → 에러 없이 성공 (P1) -- [ ] 마감 후 취소 시도 → 차단 (P0) +- [x] confirmation_code + 이름으로 조회 성공 +- [x] 잘못된 정보로 조회 → 에러 메시지 +- [x] 취소 성공 → status 변경, canceled_at 기록 +- [x] 이미 취소된 신청 다시 취소 → 에러 없이 성공 (P1) +- [x] 마감 후 취소 시도 → 차단 (P0) **완료 조건:** 조회/취소 플로우 동작, 멱등성 보장 -- Commits: +- Commits: 830d05d, 1b27243, 4f0101d, ee8c85f --- diff --git a/TECHSPEC.md b/TECHSPEC.md index 983237d..e81ce3a 100755 --- a/TECHSPEC.md +++ b/TECHSPEC.md @@ -709,7 +709,7 @@ end ```ruby # RegistrationsController def cancel - registration = Registration.find_by!(confirmation_code: params[:code]) + registration = Registration.find_by!(confirmation_code: params[:confirmation_code], name: params[:name]) registration.cancel! redirect_to registration_path(registration), notice: "신청이 취소되었습니다." @@ -722,7 +722,7 @@ end **규칙:** -- 조회 페이지에서 취소 진행 → confirmation_code만으로 식별 +- 조회 페이지에서 취소 진행 → confirmation_code + name으로 본인 확인 - 이미 canceled/refunded 상태면 성공 응답 (에러 아님) - 신청 마감일 이후에는 취소 불가 - 서버에서 취소 가능 여부 검증 (프론트엔드 버튼 숨김에만 의존하지 않음) diff --git a/app/controllers/lookup_controller.rb b/app/controllers/lookup_controller.rb new file mode 100644 index 0000000..da68fd7 --- /dev/null +++ b/app/controllers/lookup_controller.rb @@ -0,0 +1,31 @@ +class LookupController < ApplicationController + def new + end + + def create + @registration = Registration.find_by( + confirmation_code: params[:confirmation_code], + name: params[:name] + ) + + if @registration + render :show + else + redirect_to lookup_path, alert: "신청 내역을 찾을 수 없습니다." + end + end + + def cancel + registration = Registration.find_by!( + confirmation_code: params[:confirmation_code], + name: params[:name] + ) + + registration.cancel! + redirect_to lookup_path, notice: "신청이 취소되었습니다." + rescue ActiveRecord::RecordNotFound + redirect_to lookup_path, alert: "신청 내역을 찾을 수 없습니다." + rescue Registration::NotCancelableError => e + redirect_to lookup_path, alert: e.message + end +end diff --git a/app/models/registration.rb b/app/models/registration.rb index 5bd0359..3833acb 100644 --- a/app/models/registration.rb +++ b/app/models/registration.rb @@ -1,10 +1,13 @@ class Registration < ApplicationRecord + class NotCancelableError < StandardError; end + belongs_to :race belongs_to :course before_validation :set_race_from_course before_create :generate_confirmation_code + enum :status, { applied: "applied", canceled: "canceled", refunded: "refunded" } enum :gender, { male: "male", female: "female" } normalizes :name, with: ->(name) { name.gsub(/\s+/, "") } @@ -18,6 +21,18 @@ class Registration < ApplicationRecord validates :birth_date, :gender, presence: true validates :address, presence: true, length: { maximum: 30 } + def cancelable? + applied? && !race.registration_closed? + end + + def cancel! + return true if canceled? + + raise NotCancelableError, "취소 가능 기간이 지났습니다." unless cancelable? + + update!(status: :canceled, canceled_at: Time.current) + end + private def set_race_from_course diff --git a/app/views/home/show.html.erb b/app/views/home/show.html.erb index 7b3f205..a4cd337 100644 --- a/app/views/home/show.html.erb +++ b/app/views/home/show.html.erb @@ -9,3 +9,5 @@ <% end %> + +<%= link_to "신청 조회/취소", lookup_path %> diff --git a/app/views/lookup/new.html.erb b/app/views/lookup/new.html.erb new file mode 100644 index 0000000..6202083 --- /dev/null +++ b/app/views/lookup/new.html.erb @@ -0,0 +1,13 @@ +
<%= @registration.confirmation_code %>
+이름: <%= @registration.name %>
+코스: <%= @registration.course.name %>
+상태: <%= @registration.status %>
+ +<% if @registration.cancelable? %> + <%= button_to "신청 취소", lookup_path, method: :delete, params: { confirmation_code: @registration.confirmation_code, name: @registration.name } %> +<% end %> diff --git a/config/routes.rb b/config/routes.rb index 60bfd32..4ff2634 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -11,6 +11,10 @@ root "home#show" + get "lookup", to: "lookup#new" + post "lookup", to: "lookup#create" + delete "lookup", to: "lookup#cancel" + resources :courses, only: [] do resources :registrations, only: [ :new, :create, :show ] end diff --git a/docs/resume.md b/docs/resume.md index edcec5e..18e32e3 100644 --- a/docs/resume.md +++ b/docs/resume.md @@ -142,3 +142,117 @@ end ``` 컨트롤러와 폼에서 race를 명시적으로 다룰 필요가 없어지고, 코스 선택만으로 신청이 완성된다. + +--- + +# 취소의 멱등성 설계 + +## 문제 + +사용자가 취소 버튼을 두 번 누르거나, 완료 후 새로고침하면 같은 취소 요청이 중복으로 들어온다. 이때 에러를 보여주면 사용자는 "취소가 안 된 건가?"라고 혼란스러워한다. + +## 해결: 이미 처리된 상태면 성공 반환 + +```ruby +def cancel! + return true if canceled? + raise NotCancelableError, "취소 가능 기간이 지났습니다." unless cancelable? + update!(status: :canceled, canceled_at: Time.current) +end +``` + +- `canceled?` → 이미 원하는 결과(취소)가 달성된 상태이므로 에러 없이 `true` 반환 +- `applied?` + 마감 전 → 정상 취소 처리 +- `applied?` + 마감 후 → `NotCancelableError` (이 경우만 에러) + +에러를 던지는 기준은 "사용자의 의도를 달성할 수 없는 경우"뿐이다. + +> `refunded` 상태의 멱등성 처리는 결제 기능 도입 후 작성 예정. + +--- + +# render vs redirect: 에러 전달 방식의 차이 + +## render — 같은 요청에서 뷰를 다시 그린다 + +- 인스턴스 변수(`@registration.errors`)가 유지되므로 뷰에서 필드별 에러를 직접 표시 +- 폼 입력값이 보존됨 +- 용도: validation 실패 (필수 필드 누락, 형식 오류 등) + +```ruby +rescue ActiveRecord::RecordInvalid => e + @registration = e.record + render :new, status: :unprocessable_entity +``` + +## redirect — 새 요청을 보낸다 + +- 인스턴스 변수가 소멸하므로 `flash`로 메시지를 전달 +- 폼 입력값이 사라짐 +- 용도: 권한 거부, 중복 신청, 마감 등 폼 보존이 불필요한 경우 + +```ruby +redirect_to new_course_registration_path(@course), alert: "정원이 마감되었습니다." +``` + +## 테스트에서의 차이 + +| 방식 | 응답 검증 | 메시지 검증 | +|------|-----------|-------------| +| render | `assert_response :unprocessable_entity` | `assert_select ".field-errors"` | +| redirect | `assert_redirected_to path` | `assert_equal "메시지", flash[:alert]` | + +redirect 후에는 페이지 본문이 없으므로 `assert_select`를 쓸 수 없고, flash로 검증한다. + +--- + +# Turbo Drive와 POST → render 충돌 + +일반적인 Rails POST 흐름은 "데이터 생성 → redirect → 새 페이지"이다. Turbo Drive는 이를 전제로 설계되어, POST 응답이 200(render)이면 에러를 발생시킨다. redirect(303) 또는 4xx/5xx만 허용한다. + +**문제:** lookup은 POST가 "생성"이 아니라 "검색"이다. 결과를 그 자리에서 render하고 싶지만, Turbo가 이를 거부한다. redirect하려면 URL에 confirmation_code와 이름을 실어야 하는데, 이는 개인정보 노출이므로 안 된다. + +**해결:** 해당 폼에서만 Turbo를 비활성화한다. + +```erb +<%= form_with url: lookup_path, method: :post, data: { turbo: false } do |f| %> +``` + +--- + +# if/else vs rescue: 에러 처리 방식 선택 기준 + +- **if/else** — 예상된 분기. 정상 흐름의 일부. (예: 조회 실패, 사전 체크) +- **rescue** — 예외 상황. 정상 흐름에서 벗어난 경우. (예: 동시성 충돌, DB 제약 위반) + +--- + +# Rails normalizes vs before_validation + +## normalizes — 선언적 정규화 (Rails 7.1+) + +```ruby +normalizes :name, with: ->(name) { name.gsub(/\s+/, "") } +normalizes :phone_number, with: ->(phone) { phone.gsub(/\D/, "") } +``` + +- 속성이 **대입되는 즉시** 변환됨 (`registration.name = "홍 길 동"` → 바로 `"홍길동"`) +- `find_by`에도 정규화가 적용됨 (`Registration.find_by(name: "홍 길 동")` → `"홍길동"`으로 검색) +- 선언적이라 의도가 명확하고, 콜백 순서에 의존하지 않음 + +## before_validation — 콜백 기반 정규화 + +```ruby +before_validation :normalize_name +def normalize_name + self.name = name.gsub(/\s+/, "") +end +``` + +- `valid?`나 `save` 호출 시에만 변환됨 — 대입 직후에는 원본값이 남아 있음 +- `find_by`에 적용되지 않음 — 검색 시 수동으로 정규화해야 함 +- 콜백 순서에 따라 다른 validation과 충돌할 수 있음 + +## 이 프로젝트에서의 선택 + +이름과 전화번호에 `normalizes`를 사용하여, 저장뿐 아니라 조회 시에도 일관된 정규화를 보장한다. 특히 조회 기능(`find_by(name:)`)에서 사용자가 공백을 포함해 입력해도 정확히 매칭된다. diff --git a/test/fixtures/registrations.yml b/test/fixtures/registrations.yml index 9b528a9..90f77c2 100644 --- a/test/fixtures/registrations.yml +++ b/test/fixtures/registrations.yml @@ -7,3 +7,13 @@ hong_5km: phone_number: "01012345678" address: "서울시 강남구" confirmation_code: "A1B2C3D4" + +closed_registration: + race: closed_race + course: closed_five_km + name: "김마감" + birth_date: "1985-05-05" + gender: "female" + phone_number: "01055556666" + address: "부산시 해운대구" + confirmation_code: "X9Y8Z7W6" diff --git a/test/integration/lookup_test.rb b/test/integration/lookup_test.rb new file mode 100644 index 0000000..102e901 --- /dev/null +++ b/test/integration/lookup_test.rb @@ -0,0 +1,65 @@ +require "test_helper" + +class LookupTest < ActionDispatch::IntegrationTest + test "lookup with valid confirmation_code and name shows registration details" do + registration = registrations(:hong_5km) + + post lookup_path, params: { + confirmation_code: registration.confirmation_code, + name: registration.name + } + + assert_response :success + assert_select ".confirmation-code", text: /#{registration.confirmation_code}/ + end + + test "lookup with invalid information shows error message" do + post lookup_path, params: { + confirmation_code: "ZZZZZZZZ", + name: "없는사람" + } + + assert_redirected_to lookup_path + assert_equal "신청 내역을 찾을 수 없습니다.", flash[:alert] + end + + test "cancel changes status to canceled and records canceled_at" do + registration = registrations(:hong_5km) + + delete lookup_path, params: { + confirmation_code: registration.confirmation_code, + name: registration.name + } + + registration.reload + assert registration.canceled? + assert_not_nil registration.canceled_at + assert_redirected_to lookup_path + assert_equal "신청이 취소되었습니다.", flash[:notice] + end + + test "canceling already canceled registration succeeds without error" do + registration = registrations(:hong_5km) + registration.cancel! + + delete lookup_path, params: { + confirmation_code: registration.confirmation_code, + name: registration.name + } + + assert_redirected_to lookup_path + assert_equal "신청이 취소되었습니다.", flash[:notice] + end + + test "canceling after registration deadline is blocked" do + registration = registrations(:closed_registration) + + delete lookup_path, params: { + confirmation_code: registration.confirmation_code, + name: registration.name + } + + assert_redirected_to lookup_path + assert_equal "취소 가능 기간이 지났습니다.", flash[:alert] + end +end diff --git a/test/models/registration_test.rb b/test/models/registration_test.rb index f84c267..17ff294 100644 --- a/test/models/registration_test.rb +++ b/test/models/registration_test.rb @@ -76,6 +76,42 @@ class RegistrationTest < ActiveSupport::TestCase assert_match(/\A[A-Z0-9]{8}\z/, registration.confirmation_code) end + test "cancelable? returns true when applied and registration open" do + assert registrations(:hong_5km).cancelable? + end + + test "cancelable? returns false when registration closed" do + assert_not registrations(:closed_registration).cancelable? + end + + test "cancelable? returns false when canceled or refunded" do + registration = registrations(:hong_5km) + + registration.status = :canceled + assert_not registration.cancelable? + + registration.status = :refunded + assert_not registration.cancelable? + end + + test "cancel! changes status to canceled and sets canceled_at" do + registration = registrations(:hong_5km) + registration.cancel! + assert registration.canceled? + assert_not_nil registration.canceled_at + end + + test "cancel! is idempotent for already canceled registration" do + registration = registrations(:hong_5km) + registration.cancel! + assert registration.cancel! + end + + test "cancel! raises NotCancelableError when registration closed" do + registration = registrations(:closed_registration) + assert_raises(Registration::NotCancelableError) { registration.cancel! } + end + test "requires name, phone_number, birth_date, gender, and address" do registration = registrations(:hong_5km) registration.name = nil