From 946785482853911bf06f910e336a987a9e50c97f Mon Sep 17 00:00:00 2001 From: oleghasjanov Date: Fri, 6 Dec 2024 12:25:54 +0200 Subject: [PATCH] Add user_unique_id to free domain reservation - Add user_unique_id to ReservedDomain.reserve_domains_without_payment response - Create FreeDomainReservationHolder with unique ID for free domain reservations - Add error handling for case when no domains are available - Update tests to cover new functionality and error cases - Increase unique ID length from 8 to 10 characters This change adds tracking of free domain reservations through unique IDs and improves error handling when no domains are available for reservation. --- .../reserve_domains_controller.rb | 3 +- app/models/free_domain_reservation_holder.rb | 15 ++++++ app/models/reserve_domain_invoice.rb | 2 +- app/models/reserved_domain.rb | 11 ++-- ..._create_free_domain_reservation_holders.rb | 9 ++++ db/structure.sql | 50 ++++++++++++++++++- .../reserve_domains_controller_test.rb | 18 ++++++- .../free_domain_reservation_holder_test.rb | 33 ++++++++++++ test/models/reserved_domain_test.rb | 24 +++++++++ 9 files changed, 157 insertions(+), 8 deletions(-) create mode 100644 app/models/free_domain_reservation_holder.rb create mode 100644 db/migrate/20241206085817_create_free_domain_reservation_holders.rb create mode 100644 test/models/free_domain_reservation_holder_test.rb diff --git a/app/controllers/api/v1/business_registry/reserve_domains_controller.rb b/app/controllers/api/v1/business_registry/reserve_domains_controller.rb index 5f3721cd7..4c26dfb45 100644 --- a/app/controllers/api/v1/business_registry/reserve_domains_controller.rb +++ b/app/controllers/api/v1/business_registry/reserve_domains_controller.rb @@ -17,7 +17,8 @@ def create password: domain.password, expire_at: domain.expire_at } - end + end, + user_unique_id: result.user_unique_id }, :created) else render_error(result.errors, :unprocessable_entity) diff --git a/app/models/free_domain_reservation_holder.rb b/app/models/free_domain_reservation_holder.rb new file mode 100644 index 000000000..6d7c316bc --- /dev/null +++ b/app/models/free_domain_reservation_holder.rb @@ -0,0 +1,15 @@ +class FreeDomainReservationHolder < ApplicationRecord + before_validation :set_user_unique_id + validates :user_unique_id, presence: true, uniqueness: true + + def reserved_domains + ReservedDomain.where(name: domain_names) + end + + private + + def set_user_unique_id + self.user_unique_id = SecureRandom.uuid[0..9] + end + +end diff --git a/app/models/reserve_domain_invoice.rb b/app/models/reserve_domain_invoice.rb index 51a685e6e..0fc467408 100644 --- a/app/models/reserve_domain_invoice.rb +++ b/app/models/reserve_domain_invoice.rb @@ -131,7 +131,7 @@ def build_invoice(attributes) end def generate_unique_id - SecureRandom.uuid[0..7] + SecureRandom.uuid[0..9] end def process_invoice(invoice) diff --git a/app/models/reserved_domain.rb b/app/models/reserved_domain.rb index d379012d7..abde7cd2c 100644 --- a/app/models/reserved_domain.rb +++ b/app/models/reserved_domain.rb @@ -43,13 +43,13 @@ def new_password_for(name) record.save end - def wrap_reserved_domains_to_struct(reserved_domains, success, errors = nil) - Struct.new(:reserved_domains, :success, :errors).new(reserved_domains, success, errors) + def wrap_reserved_domains_to_struct(reserved_domains, success, user_unique_id = nil, errors = nil) + Struct.new(:reserved_domains, :success, :user_unique_id, :errors).new(reserved_domains, success, user_unique_id, errors) end def reserve_domains_without_payment(domain_names) if domain_names.count > MAX_DOMAIN_NAME_PER_REQUEST - return wrap_reserved_domains_to_struct(domain_names, false, "The maximum number of domain names per request is #{MAX_DOMAIN_NAME_PER_REQUEST}") + return wrap_reserved_domains_to_struct(domain_names, false, nil, "The maximum number of domain names per request is #{MAX_DOMAIN_NAME_PER_REQUEST}") end available_domains = BusinessRegistry::DomainAvailabilityCheckerService.filter_available(domain_names) @@ -65,7 +65,10 @@ def reserve_domains_without_payment(domain_names) reserved_domains << reserved_domain end - wrap_reserved_domains_to_struct(reserved_domains, true) + return wrap_reserved_domains_to_struct(reserved_domains, false, nil, "No available domains") if reserved_domains.empty? + + unique_id = FreeDomainReservationHolder.create!(domain_names: available_domains).user_unique_id + wrap_reserved_domains_to_struct(reserved_domains, true, unique_id) end end diff --git a/db/migrate/20241206085817_create_free_domain_reservation_holders.rb b/db/migrate/20241206085817_create_free_domain_reservation_holders.rb new file mode 100644 index 000000000..a9aade380 --- /dev/null +++ b/db/migrate/20241206085817_create_free_domain_reservation_holders.rb @@ -0,0 +1,9 @@ +class CreateFreeDomainReservationHolders < ActiveRecord::Migration[6.1] + def change + create_table :free_domain_reservation_holders do |t| + t.string :user_unique_id, null: false, unique: true + t.string :domain_names, array: true, default: [] + t.timestamps + end + end +end diff --git a/db/structure.sql b/db/structure.sql index 5aa885be2..6d815b223 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -1168,6 +1168,38 @@ CREATE SEQUENCE public.epp_sessions_id_seq ALTER SEQUENCE public.epp_sessions_id_seq OWNED BY public.epp_sessions.id; +-- +-- Name: free_domain_reservation_holders; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE public.free_domain_reservation_holders ( + id bigint NOT NULL, + user_unique_id character varying NOT NULL, + domain_names character varying[] DEFAULT '{}'::character varying[], + created_at timestamp(6) without time zone NOT NULL, + updated_at timestamp(6) without time zone NOT NULL +); + + +-- +-- Name: free_domain_reservation_holders_id_seq; Type: SEQUENCE; Schema: public; Owner: - +-- + +CREATE SEQUENCE public.free_domain_reservation_holders_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + + +-- +-- Name: free_domain_reservation_holders_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: - +-- + +ALTER SEQUENCE public.free_domain_reservation_holders_id_seq OWNED BY public.free_domain_reservation_holders.id; + + -- -- Name: invoice_items; Type: TABLE; Schema: public; Owner: - -- @@ -3186,6 +3218,13 @@ ALTER TABLE ONLY public.epp_logs ALTER COLUMN id SET DEFAULT nextval('public.epp ALTER TABLE ONLY public.epp_sessions ALTER COLUMN id SET DEFAULT nextval('public.epp_sessions_id_seq'::regclass); +-- +-- Name: free_domain_reservation_holders id; Type: DEFAULT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.free_domain_reservation_holders ALTER COLUMN id SET DEFAULT nextval('public.free_domain_reservation_holders_id_seq'::regclass); + + -- -- Name: invoice_items id; Type: DEFAULT; Schema: public; Owner: - -- @@ -3715,6 +3754,14 @@ ALTER TABLE ONLY public.epp_sessions ADD CONSTRAINT epp_sessions_pkey PRIMARY KEY (id); +-- +-- Name: free_domain_reservation_holders free_domain_reservation_holders_pkey; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.free_domain_reservation_holders + ADD CONSTRAINT free_domain_reservation_holders_pkey PRIMARY KEY (id); + + -- -- Name: invoice_items invoice_items_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- @@ -5669,6 +5716,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20241104104620'), ('20241112093540'), ('20241112124405'), -('20241129095711'); +('20241129095711'), +('20241206085817'); diff --git a/test/integration/api/business_registry/reserve_domains_controller_test.rb b/test/integration/api/business_registry/reserve_domains_controller_test.rb index 1de301d26..945b4706a 100644 --- a/test/integration/api/business_registry/reserve_domains_controller_test.rb +++ b/test/integration/api/business_registry/reserve_domains_controller_test.rb @@ -30,7 +30,7 @@ def setup test "should reserve multiple domains successfully" do domain_names = ["new1.test", "new2.test"] - assert_difference 'ReservedDomain.count', 2 do + assert_difference ['ReservedDomain.count'], 2 do post api_v1_business_registry_reserve_domains_path, params: { domain_names: domain_names }, headers: @auth_headers @@ -40,6 +40,8 @@ def setup json_response = JSON.parse(response.body) assert_equal "Domains reserved successfully", json_response['message'] assert_equal 2, json_response['reserved_domains'].length + assert_not_nil json_response['user_unique_id'] + assert_equal 10, json_response['user_unique_id'].length json_response['reserved_domains'].each do |domain| assert domain_names.include?(domain['name']) @@ -127,4 +129,18 @@ def setup expire_at = Time.parse(domain['expire_at']) assert_in_delta Time.current + ReservedDomain::FREE_RESERVATION_EXPIRY, expire_at, 5.seconds end + + test "should return error when no domains are available" do + domain_names = ["new1.test", "new2.test"] + + BusinessRegistry::DomainAvailabilityCheckerService.stub :filter_available, [] do + post api_v1_business_registry_reserve_domains_path, + params: { domain_names: domain_names }, + headers: @auth_headers + + assert_response :unprocessable_entity + json_response = JSON.parse(response.body) + assert_equal "No available domains", json_response['error'] + end + end end \ No newline at end of file diff --git a/test/models/free_domain_reservation_holder_test.rb b/test/models/free_domain_reservation_holder_test.rb new file mode 100644 index 000000000..6aae40295 --- /dev/null +++ b/test/models/free_domain_reservation_holder_test.rb @@ -0,0 +1,33 @@ +require 'test_helper' + +class FreeDomainReservationHolderTest < ActiveSupport::TestCase + def setup + @holder = FreeDomainReservationHolder.create!(domain_names: ['example1.test', 'example2.test']) + end + + test "should be valid with valid attributes" do + holder = FreeDomainReservationHolder.new( + domain_names: ['example1.test', 'example2.test'] + ) + assert holder.valid? + end + + test "should generate user_unique_id before create" do + holder = FreeDomainReservationHolder.create( + domain_names: ['example.test'] + ) + assert_not_nil holder.user_unique_id + assert_equal 10, holder.user_unique_id.length + end + + + test "should have many reserved domains" do + holder = FreeDomainReservationHolder.create( + domain_names: ['example.test'] + ) + reserved_domain = ReservedDomain.create( + name: 'example.test', + ) + assert_includes holder.reserved_domains, reserved_domain + end +end \ No newline at end of file diff --git a/test/models/reserved_domain_test.rb b/test/models/reserved_domain_test.rb index ba6b3a13e..bb7459ff9 100644 --- a/test/models/reserved_domain_test.rb +++ b/test/models/reserved_domain_test.rb @@ -66,6 +66,7 @@ class ReservedDomainTest < ActiveSupport::TestCase result = ReservedDomain.reserve_domains_without_payment(domain_names) assert_not result.success + p result assert_includes result.errors, "The maximum number of domain names per request is #{ReservedDomain::MAX_DOMAIN_NAME_PER_REQUEST}" end @@ -131,4 +132,27 @@ class ReservedDomainTest < ActiveSupport::TestCase @reserved_domain.destroy_if_expired end end + + test "reserve_domains_without_payment should create holder and return unique_id" do + domain_names = ['test1.test', 'test2.test'] + + result = ReservedDomain.reserve_domains_without_payment(domain_names) + + assert result.success + assert_not_nil result.user_unique_id + assert_equal 10, result.user_unique_id.length + assert_equal domain_names.count, result.reserved_domains.count + end + + test "reserve_domains_without_payment should return error when no domains available" do + domain_names = ['test1.test'] + + BusinessRegistry::DomainAvailabilityCheckerService.stub :filter_available, [] do + result = ReservedDomain.reserve_domains_without_payment(domain_names) + + assert_not result.success + assert_nil result.user_unique_id + assert_equal "No available domains", result.errors + end + end end