From d904f817a323e1deb3c934a771c4489ae2424a05 Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Sat, 27 Jan 2024 14:47:21 +0100 Subject: [PATCH 1/7] Model test for images (passes) --- spec/fabricators/image_fabricator.rb | 8 ++++++++ spec/models/space_spec.rb | 10 ++++++++++ spec/rails_helper.rb | 3 +++ spec/support/test_image/test_image.jpg | Bin 0 -> 8528 bytes spec/support/test_image/test_image_helper.rb | 19 +++++++++++++++++++ 5 files changed, 40 insertions(+) create mode 100644 spec/fabricators/image_fabricator.rb create mode 100644 spec/support/test_image/test_image.jpg create mode 100644 spec/support/test_image/test_image_helper.rb diff --git a/spec/fabricators/image_fabricator.rb b/spec/fabricators/image_fabricator.rb new file mode 100644 index 00000000..171e619b --- /dev/null +++ b/spec/fabricators/image_fabricator.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +Fabricator(:image) do + space + after_create do |image| + image.image.attach(TestImageHelper.image_upload) + end +end diff --git a/spec/models/space_spec.rb b/spec/models/space_spec.rb index ec0a6491..c20efbaa 100644 --- a/spec/models/space_spec.rb +++ b/spec/models/space_spec.rb @@ -107,4 +107,14 @@ expect(space.versions.last.event).to eq("update") end end + + describe "Image upload" do + let(:space) { Fabricate(:space) } + let(:image) { Fabricate(:image, space:) } + + it "allows image to be attached" do + expect(image.image).to be_attached + expect(space.images.count).to eq(1) + end + end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 6ecd72f5..9116b2a8 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -8,6 +8,7 @@ # Add additional requires below this line. Rails is not loaded until this point! require 'selenium-webdriver' require 'vcr' +require 'support/test_image/test_image_helper' # Requires supporting ruby files with custom matchers and macros, etc, in @@ -119,4 +120,6 @@ (uri.host == 'localhost' && uri.port != 3001) # Ignore localhost except on port 3001 which is used for recording KRA API cassettes end end + + config.include TestImageHelper end diff --git a/spec/support/test_image/test_image.jpg b/spec/support/test_image/test_image.jpg new file mode 100644 index 0000000000000000000000000000000000000000..3863ef739253d3d4d2fd8fb87874295b04c64e6d GIT binary patch literal 8528 zcmb8SWmMG98#calmvn>Fl9JNh4ZE(a)HIA7jLfX8?Ck8c44mAYY}_nt>}>xN!6PIhA|@iH zBq5<>qobi?`+vsWM*tlu9smzOfcFr9Plrc9hj%v&-~<5h2nq1;0RKPm0eJU5iAhMw z?!%91?_2)^J`v$P@1BGQz`qaE5%Wqt(6l6>cVT!)$|oHh%czxIF(3nSZJ*sby&uQ^ zKN{ZuB})LnBO)dwxerRv0r2lt@reP%1pnW}c=w`oL=SkSG>PdSTDk-?#Aa8_o=R&C zY~8H@DDPSLbOdw&b-=WA@pfa|$1qIiiiyH5`F7|k(I`4S-PcxllvvGr>M_{Zl8VDbd#5C~Q!E-DEjP(fWV@%IoLrsoPP|PQ>TRvSV-3Bu&SnxBN#WY*?kr zPn#5JXX`GW2~2P!GFy&&kmJz(-EW&85qVj<8upS}iJw^@6T}BXl5UBrx;YdYm|BY&dkwpX1L%|y zY;mIGkewt5yeraIyWI3AHd&dF+wAl6th^*8MJcC6jxbRBd#e^tuI)qDQ{BAfF!_*L zXb8RDnund6q-6d}XQ|?P`?=KS9ix$l8k8gTY{UM(+08gM0-hv3=wrRbdV(+$jcmE1 z%DuB1DE;|i`oj(_wiL-P_1c!iXQ}l3n)O4GAV3*MB`>#;qloi-TT@fkdCgqSzJZ(_ zDLTw_4z-UH3ur3frCLVqgWcReH89zT?~wTxcaixf=KRDkOWE#g*;ho0Fntq|{^Z@0 z!|Gc;Yt>f1w__)M_EH|mGXMEW{IPQNe2Dz9aQ<4;Sgs-0j;-sf$Hz@liE@C!KmnuA z)r&6@MOiy!6WdqFK=KJkfxwmF;g@4^yn~u`@nI$gbTYtqwNzR%3vrC+JesryaeItW})s4qsS2b$s*E>h$$MDr0XylPAq1ini6P%s-XHux%fB^2!kp@)Q*bdoP4noa( z*@t*i4@u<|IBF8Ugrq-A!c!poXr5uoui2JMzoi~($UscLrAA*LhnJ4PV|r{%eySmb z@#7n`1rzd(0)whYh1)6s`#WUhCLsxlw&HYjn6xevEqvNE9f3LmtE(LnBL{8Eq=>az z=fUtEYGFySYGzkYQ8DsiUm#3gE&KICbt3^jRY@x}F z9AI|Me?@DC&}0LK{sRko#f4fze?`4PdG4k{c!0;1W^Uxx8;Ct+t8dxx^J-LTkV zMq*y`@gWI?X|p?MXIVM6NJ(cB+Ea!;APTDfR{!cVTR_AQ$dHJH0c~rYaZ)^XqFVQP zq71N3`N|(s!XTbDlR^#Qv-Y*8;`Yhig6B~NOuD@QuSIxs*>F|5-v@0cJ8ND$}5RhGi1TpxS#Zd37>R7d-jSAQq{MY(M0l+;8t30MWi}_C$BxWVKEfso&fIkU*7krr9wk_c z$l$8gr6dU&xdXs5kI)~XGEMiknkb7v2~?`m3nxBXM{k z@Wx>oPI;U(9a7tn^I>@@@UJMdzl~S>=dqS0HeW;cTXBR2*+1rFaizjC6Y1%vwLX#t;rQqSlC@!*9)HH_)q1$m*Wwe_ol^(6`e7P-R@*H?aKXQDRCId z!0wf!_tUcK=BM-f?hmFG-y8*gcnR@;*3nb=+HO1%_H+!Lj<%apyism1eB3KF?H&Au z9-YzMO#zoV5cP1UZS$B8{z&mTl=)y{-Q@Se5{WLz-W^(OwwpDZL4P3bANC$RH)CV; zlu}5M9bPK$^mJ6Ex9%)PM2))fJ!@Ut&;E=ur`<|%r3P#y@>5__h9SvCN$PD!XEXQ* zs$lN2Kpaj}h^pkn3MZB)akEZ~BK2(;Ia?2eMFd`b9lb55bar#g8H7T;YTlcwPJRIM z=8yBaPk#?AJGJQ^g!PB4kEp}O8@w4hud{>S1jm#;X}yZNF*&&LY^ar9silT#NTjbS zmS2u$BCW4m)-|oq&n6dux2Dch>hP`C-1W?+$}#YfkxQ+?B}omZCyQ-&fPmZNzY_b^ zT|}IY4Q%LFNelDyvUzajAamHS;;MJQnkx(SY3dd2zPV1la6>O1M8T=vL5!7SYB^-7 z4SyzraLwXNtUCU%N>~Q_qEybH9DjJWjk)cLeX@hwv>lZK>%YSnm`R6#`IZ_eG3BR^ zUAK<8;a?AAYdB=M!WYWF<-4fEW&qp%gSJ*;2sG4DW;-05<|gpe{%2HWK}RV>Z&Wx% zx7b5!OCy_=9VwLHg~m^C1!RB$sgjy6XJemlSGsypP0_czS8N}tG}4vigi%}P-v|yn zi0N66MQmB61T_E4FTq~7&l?z%OzxZB z>C>J=7q+m}cd+T%3bUXZ%JCv<#HDMT;P98dQkmjo>TrtSrdq#q8T}6q%l4dQqJ|wH zYoFHO9$*O_zcMvNq(9vHS`BSmkUK)4fmcP~-1aD3@BP*JrCNVAqzYau5|X=31U zc;g4-vL%O zViX%+;(RK-j6KpTTD`X!`!)hQyNykL!>9a$hUdN-to@bn{o(K-2fO@pnoOd0QesaD zUSsN&6n!l#2f3-*6EUnUR5S@VP}mPrP_&EkOe=hA54Pi0aSSmc*iPI!5+5-nj24DF zYIh~o_doIPY!#2r$eDB&s;NRizU@CxZfb1ldMcJfSsx9{H5uh#N#Xn_X*1IzQDcN# zc(W@0q3UB-!s)cJ&x!eAcafD<&}v1rpZ7$ybFk2qIh-gg?X8vKd25m($7M3DyMkLZ zQ4jcW`4ERoUsn3h5lHE3@pP{-$k8g-dY z3NDGR*{GZP%Q#eLtOKg*_b0gKg3GekJ!OyL;tFw`>=j8CECknuG?vFk%?&XcZ}*Fg z8(J8}SOS+vIQn+KNVx2OXtD}d)$kV4-v6XM-Lc#63Tp%IuWUSVz@XUEBLA71s~t3R zOTZ$Y)VxtW&?PFuHtKqOFU*>Ue2ziex^Dp2BEyuwc;Q8cw2=Qur$1o~DiC&j)a@?5 ztg!thdMk>1!BkPO4=WhA=myuwZiSYHu}rEVr?4%nKd!dOjjYt1qagXJsc95d(8~= zP(EV^`Q8Tj2iFHS-rCM%(xkmLW~sb!+}@b3*Im)<$WsjWN>-u1PlQ?>^9t{ zVhvVKi@43}x16tA=&DbZ%gH=hUOkcsqfZOqL*BI5ka+sh-AWxmjSarEGF}(hK3tL~ zr?58OuX|rb2zO~#+|0l^O(6=tr;Hw}P#43)ophI02uB+ZzFjZ=w;mZ)sPsB@^F_|Y zRY>UGV)2171@~fvzgSI-gNMQ~;x?oHDsUeD%Vyng!Mei+^iioII2fi!EiHq4CKVj0 z(F8eC*9Mz?A6xBDa=&zlvs?VdFjJM6Q1qb+5NTzy;_}QsrG0fkM z<7=yVhvl3Om{rN0Tbbr}PRpJzh8)P3{@Klq6}r5JuW||q$v0oL98PlfF+r5DUF5mf=U(%O1E?p=&J zi%<_!Q!!5)oxzy4-kZWwc=UkF`QK^r+u=Y74;44DpPs!PR^Bl#@SatXr{*SK?FW~# zp0X!BGgD;$cIEi!bVnsL$A;7^%J{oD%=`!!(rg}7*E0VLvVm8qZ#;jo%kHJ%;8imT zKY2v$F-M9}t0gbD9l?g?kT6CcQ4NPWz16 zK(N2Y`{Q3~Tfnm?yiSJWH+xTHJ;~6I_?^+%^iQ zy@Ba~=y{kuTNpwdZZ^6cV&rg>K(ztJo_Krh$ zMddA3c8kN6CYpV*R^=7`OjVC;R736n;Q7)Y4ZL{$R~(O&o+bOVU~P;V6QcwCKql^f z`D1GrDv$IXb`wuYXnN_|iGnwW6Mm^13=s)A6kf+39NP?A~dVke$T$noA zaVpapx0Csv6c%R|jO|!D$z9E%*I&?|Wfh|K=52Ab60vAa0Yi5#6Slpo;kwYAq;FU6Dn;CM>7C z-vW7tc8CrIS)S*jRWc)zz3Nx*0A$S1qTMeg{`x^;YrFxtn5#%2sUSkH@OE_9xeJB zccFYES8xSC3QqT+w{)316V9`nsS=VaS-=X z_W!X{rO_ltF1qdUWOawLl(62KDsDl0{&>zY)A(-VgsZ6q`h0wD2fHe}O88eWv1elx zPYfi4ZZavFO4w5^O6S+uUg*!jS@$pR-}4?E-xAQfSE|}7*inr?!j;kn>~Cepb)n*Z z&b^c+)5lG7^E%g(Tk+i?z1YtXI>bILyjiz?MZd z)KXJsjrGT;7OUUKz0hBD{Dcr~Qa_$KIJPf@W!=_CYF2f0)`3w49keWue{+ z9v2tojh;us3)L!1+S|;=){|o|Inv|StEvaB7I%x^2f?o2wS>N22|E(jvh`{C6S0+b zAnu~pTz^x@YJ~k0oL#ov1gBc*phy?sR9;?21w&I+oasA;eO^~v(QB$&EBo4RF3?{P zdGfYV{1)pvT>@Jwk&Sk7Ad(b^<42e(T`0Vu zzKXIed0TH?L-J2C`dw5s*Q@T?PgFp6Dk?WN05?U(KYmnwybDdnA;Ok) zlxU-<|4(BZmegZ!1rky4Z&yL3b%TRCPMIP}@!(tWtHB-F?!C=VyH&fTwYmLa$Dlj~ zmH(hs4gN%uwVm_&_P&I1ZOH)kWxP@4ov3dnlmfJDY-&gBFkX>J`Zu+v z7g9KNhXzDDle(i!PI(OOte|2MIs>-}Wq$}Z`;f89& z^^ICmJ{Xm;7k-<|5zD5q>l?wrsy*EihpRsoU z+rG_!XgAR?&zxSLiJ_+7kK8lTsJ}eZ@Ya@auvCX*~iv%boZXb*Iyf+ zFk6kR#GkkK5`X$~>DWR#mZn-IP|K&Bxa%(rwyu8kKvxAM*HQM_?Lu75#zL*Z*1L+T zp;wn8V_jgiwb;p`8T9_z@ZAkpjOzlnFZa`abZgbrX39jOKcx2p+I}>|LKoXMuqj%0 z%PEI$nD=g>n>xttnmd=xbXm6@_;cJZ)QS#rWMx7nHI!B&yjPaDQ@56{R&xft-P}Z2 z`8u=>21NIj{N7El2rY=oow*D?Y2wnRxe zeef&@v9&8$LKR@oP#dgpoS_tT!xBU0=P`FG@>_phpFxPK>_yNwzzfU*yz4jvdOi

QdlcL;N%aIy$La|yI?9}13m3^KzK#^2>YduF95{&5 zGkf8|$zUCKdHeXAIltQC2{j95qo}4CJNqBNitFz>lQg&!XYPfRI37ZdunS28l zG9A&vS6Wt4ZZnMYa!aUfRj<7Ra4XvJ`PN+vTB{<+U7u;+xQE#?4TyJqQ;{1+>%Dtv z4u99X5y(_%4@uHDHp3q283Y&}Kex6vW7*zwYW6Zp5LIeV6Z~}$@og)r#8_h!7gPxa z1V{78t@<@xC+e!Mnp#UENG3&%pD5|G-2aOwCZ3OHR&^Kp_I?I;ZTG8Q?gTP76x8`! zgDrdEDXD9IebIwC2h%#!NMDI`qqm`oulLU*FU^)9w>rp^By<0k{U3a+|Cp;f{ol>n z+dk2hzFp-w?1dK${3iNcKr}Tm#(r$$IRjHOVlBvH$-e_kOgs1c(!Xx$RfD;IY4J+T zbn`RCv2dUUm*lfqNl9_W85Xn&u6)N?I$F-ONogwYke}7SM&ZITiOF zgN3mkBPz7|IfJ)O=W(?b)8Zo|Bbdv~$%JD1Gn$+=zCpzmN3+xEdA+f-{SVTr7Q&rZ zYR^I5zUUZ@Ytbf<##l*m`&wDzig2ihY*^NQ{X^Mm;tW(g&NXmeF4TUocg_C$*q`F> zd8c0@2JF~>AGE{`?{1wy^;iI_s(B4xzc!D2R_j%42El*WtWGIA<75-d480rl7FO&3 zb^MZfqB#|lKDOKQZ!yz`YH;Z}920zk_DM(!B3iu?<*o3wcS-MBCF7#ZIF4?*-34R! z8+y$3={=zn1*BO7ahd~8Y(?uf%oCoaZ@#jpEB!VnH~wq?LT>=oz@|I+v#x-?QN(4h z78Nm(5iY^k8FR3iTUL1V*mDabrA6JfM!Kw>cRk6T@ho6<3|^T zg?YH!lMp7OaPHo3qT+6^K2I`-=YDZ@T_*v=A5}BiUVc^Vgl^q z&D);{aptL3oTKglj@p(g3?c7XVigH;W=0NHLP6_2G-WVf#tN~vqr!iMlc>GA!}Ak) zr=G&p?eG=i=QA^EbjGj`I1!%SrH@wy_A?iX-+V~2y4cdyE%gp{26tt*hH449asH1f z1S=PQhsq+9P!-54KXJt%=jlLFA)^hdDTSda*u39@(s{SlcXc77ruR?kIwu)tJFACm zH#@X~@YaH-J>L4r*x1qYy|UvJz5J+>^TPgFjGfO6lRXf9@cbpcy2kiKD*qjT)z(y$ zD!|R3+wPEKVRFIZY{i&Hh#(Tm#>kvis-~3FrOJV{v4y^r$T^mpSSusB6@N7ZqLTS4 znaathL9NoW6F6^AT=;$!8wpLD;+^E#npA^?OjI>r#Ak6TClh@Uh5T*+8xy7{`sO2d zrl*Afi@~!KK|R<{dW#E#Fxll@{%@k{&bdjC`?}W6hMv!CiiRzg7LA9s{;TrNPRmbI zlmC~OnA-C!G8j7gfi!dc2O&?1a2?P;nx?vW8UWa5}&X2(u%VRl;Jd{$vzRhpz$5+GK+kIsLFQUa3%1l@N z(jTsyjAh=gm7R6q48pD1f*k20BR)>Mx%)sr!pwOZIxiYTZ1S*JUG~&*acA-`W(D2i zJLP%=?%K%pHwlAO%hh?Vd2n)CWd_6{2%;HjD38ox;z1a`Dvv!&24*!-;e&;Q+Ye8J6L23*vHG>w=R4jIXX6C|GpXvxS+O5S!4)y=2&06-1E%g$FF- zin+k_gh+-z8stIu6&~;|lGTg`IEzmjyQ4=Q6shxyMp=j?q>4x_9GHzy;{Hby`A5gX z4kuPYB-Y5|h|tD~)oK2%Cg(T=VG@%|Ef7U2$3;M6E!7!h>JAxDTMJ`{c>u<*bCttA zMTc?cIOzviO09~?D)bJWHOU^JE7-?Q=6={yN14@h~ z1Mzg|XrpV8IGk8Lg1QU{Yd3TRBo`{{txC&DW&ZdTOZ~Tz(%M#!v2GScA?UMXFvuZ2M|x!`dsgjrrI$}Q35><* zkxLEwIzPZq0S@R-I~Ct+m0G$Z+&S4e22o0i(Hc7m)0g-brH9nw?6Y!43&$UQ9MidC%cXSl+po|TkS9c_1G$FlwW^BuA+ WYHf|YfR`n2veESG)Ovh(xc>tvlKJHT literal 0 HcmV?d00001 diff --git a/spec/support/test_image/test_image_helper.rb b/spec/support/test_image/test_image_helper.rb new file mode 100644 index 00000000..d2ad69fe --- /dev/null +++ b/spec/support/test_image/test_image_helper.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module TestImageHelper + def self.image_path + Rails.root.join("spec/support/test_image/test_image.jpg") + end + + def self.image_file + File.open(image_path) + end + + def self.image_upload + { + io: image_file, + filename: "test_image.jpg", + content_type: "image/jpeg" + } + end +end From 03a12a270ae58a3ccc1ed4287d3b16483b5a76f1 Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Sat, 27 Jan 2024 20:38:17 +0100 Subject: [PATCH 2/7] Create request test for space images. Unfortunately this one passes too... --- config/environments/test.rb | 3 ++ spec/requests/space_images_spec.rb | 50 ++++++++++++++++++++ spec/support/test_image/test_image_helper.rb | 12 ++++- 3 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 spec/requests/space_images_spec.rb diff --git a/config/environments/test.rb b/config/environments/test.rb index 8746aa45..6bbb7f05 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -8,6 +8,9 @@ # and recreated between test runs. Don't rely on the data there! Rails.application.configure do + # Default host needed for tests: + routes.default_url_options[:host] = "localhost:3000" + # Settings specified here will take precedence over those in config/application.rb. config.cache_classes = false diff --git a/spec/requests/space_images_spec.rb b/spec/requests/space_images_spec.rb new file mode 100644 index 00000000..797286b7 --- /dev/null +++ b/spec/requests/space_images_spec.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe "SpaceImages", type: :request do + let(:user) { Fabricate(:user) } + + before do + sign_in user + end + + describe "POST /spaces/:id/upload_image" do + let(:space) { Fabricate(:space) } + let(:image_file) { fixture_file_upload(TestImageHelper.image_path, TestImageHelper.content_type) } + + it "uploads an image and redirects" do + expect do + post spaces_upload_image_path(id: space.id), params: { image: [image_file] } + end.to change(space.images, :count).by(1) + expect(response).to redirect_to(space_path(space.id)) + follow_redirect! + expect(response.body).to include(I18n.t("images.upload_success")) + end + end + + describe "PUT /spaces/:id/images/:image_id" do + let(:space) { Fabricate(:space) } + let(:image) { Fabricate(:image, space:) } + let(:params) { { image: { caption: "New Caption", credits: "New Credits" } } } + + it "updates the image and redirects" do + put(space_image_path(id: space.id, image_id: image.id), params:) + expect(response).to redirect_to(space_image_path(space)) + follow_redirect! + expect(response.body).to include(I18n.t("images.update_success")) + end + end + + describe "DELETE /spaces/:id/images/:image_id" do + let(:space) { Fabricate(:space) } + let!(:image) { Fabricate(:image, space:) } + + it "deletes the image and redirects" do + expect { delete space_image_path(id: space.id, image:) }.to change(Image, :count).by(-1) + expect(response).to redirect_to(space_image_path(space)) + follow_redirect! + expect(response.body).to include(I18n.t("images.delete_success")) + end + end +end diff --git a/spec/support/test_image/test_image_helper.rb b/spec/support/test_image/test_image_helper.rb index d2ad69fe..9f608480 100644 --- a/spec/support/test_image/test_image_helper.rb +++ b/spec/support/test_image/test_image_helper.rb @@ -5,6 +5,14 @@ def self.image_path Rails.root.join("spec/support/test_image/test_image.jpg") end + def self.content_type + "image/jpeg" + end + + def self.filename + "test_image.jpg" + end + def self.image_file File.open(image_path) end @@ -12,8 +20,8 @@ def self.image_file def self.image_upload { io: image_file, - filename: "test_image.jpg", - content_type: "image/jpeg" + filename:, + content_type: } end end From b6ff3517a9f1e4efbcf602270f395ae93cd4a715 Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Sat, 27 Jan 2024 21:47:40 +0100 Subject: [PATCH 3/7] Add validation that images are actually attached --- app/models/image.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/models/image.rb b/app/models/image.rb index 9146255f..5d829d5f 100644 --- a/app/models/image.rb +++ b/app/models/image.rb @@ -2,6 +2,8 @@ class Image < ApplicationRecord has_one_attached :image + validate :image_is_attached + belongs_to :space before_destroy :delete_image @@ -15,6 +17,10 @@ def url def delete_image image.purge end + + def image_is_attached + errors.add(:image, "must be attached") unless image.attached? + end end # == Schema Information From 2a818b028adf37bb283f0442022d744f3d233553 Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Sat, 27 Jan 2024 21:48:01 +0100 Subject: [PATCH 4/7] Fix so image fabricator attaches images on creation --- spec/fabricators/image_fabricator.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/spec/fabricators/image_fabricator.rb b/spec/fabricators/image_fabricator.rb index 171e619b..267dba2f 100644 --- a/spec/fabricators/image_fabricator.rb +++ b/spec/fabricators/image_fabricator.rb @@ -2,7 +2,5 @@ Fabricator(:image) do space - after_create do |image| - image.image.attach(TestImageHelper.image_upload) - end + image { Rack::Test::UploadedFile.new(TestImageHelper.image_path, TestImageHelper.content_type) } end From bb40f1024ba5310a6e559f66aa2d5260ee71fe72 Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Sat, 27 Jan 2024 22:01:38 +0100 Subject: [PATCH 5/7] Create failing test for the bug: It uploads an empty string as part of the image set. That string needs to be discarded. --- spec/requests/space_images_spec.rb | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/spec/requests/space_images_spec.rb b/spec/requests/space_images_spec.rb index 797286b7..6ac19c9a 100644 --- a/spec/requests/space_images_spec.rb +++ b/spec/requests/space_images_spec.rb @@ -13,9 +13,19 @@ let(:space) { Fabricate(:space) } let(:image_file) { fixture_file_upload(TestImageHelper.image_path, TestImageHelper.content_type) } + it "fails to upload an empty image" do + expect do + post spaces_upload_image_path(id: space.id), params: { image: [""] } + end.not_to change(space.images, :count) + expect(response).to redirect_to(space_path(space.id)) + follow_redirect! + expect(response.body).to include(I18n.t("images.upload_failed")) + end + it "uploads an image and redirects" do expect do - post spaces_upload_image_path(id: space.id), params: { image: [image_file] } + # Rails for some reason adds an empty "" to the start of the array when allowing multiple uploads + post spaces_upload_image_path(id: space.id), params: { image: ["", image_file] } end.to change(space.images, :count).by(1) expect(response).to redirect_to(space_path(space.id)) follow_redirect! @@ -28,7 +38,7 @@ let(:image) { Fabricate(:image, space:) } let(:params) { { image: { caption: "New Caption", credits: "New Credits" } } } - it "updates the image and redirects" do + it "updates the image info and redirects" do put(space_image_path(id: space.id, image_id: image.id), params:) expect(response).to redirect_to(space_image_path(space)) follow_redirect! From 1badca404a24f2d67dfdd3b5af85f96acc6176a5 Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Sat, 27 Jan 2024 22:02:10 +0100 Subject: [PATCH 6/7] Fix a bug that stopped image uploads --- app/controllers/space_images_controller.rb | 16 ++++++++++++++-- config/locales/nb.yml | 1 + 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/app/controllers/space_images_controller.rb b/app/controllers/space_images_controller.rb index 4ccc4f38..7ccf4968 100644 --- a/app/controllers/space_images_controller.rb +++ b/app/controllers/space_images_controller.rb @@ -19,9 +19,16 @@ def destroy end def upload_image - params["image"].each do |image| - Image.create!(space: @space, image:) + images = params["image"].compact_blank + return upload_failed if images.empty? + + # Attempt to upload each image + images.each do |image| + new_image = Image.new(space: @space, image:) + + return upload_failed unless new_image.save end + flash[:notice] = t("images.upload_success") redirect_to space_path(params[:id]) end @@ -35,4 +42,9 @@ def image_params def set_space @space = Space.find(params[:id]) end + + def upload_failed + flash[:alert] = t("images.upload_failed") + redirect_to space_path(params[:id]) + end end diff --git a/config/locales/nb.yml b/config/locales/nb.yml index 68d52a68..722c91f3 100644 --- a/config/locales/nb.yml +++ b/config/locales/nb.yml @@ -101,6 +101,7 @@ nb: images: image_missing: "Bilde mangler" upload_success: "Bilde lastet opp" + upload_failed: "Fikk ikke lastet opp bilde, noe gikk galt" upload: "Last opp" upload_image: "Last opp bilde" edit_images: "Rediger bilder" From 9a1b3a1f164c9a0b6777bf5177bc3a24f5c50b3f Mon Sep 17 00:00:00 2001 From: Daniel Jackson Date: Sat, 27 Jan 2024 22:10:43 +0100 Subject: [PATCH 7/7] Change the name of the "image" field to the more descriptive "images_to_upload", as it's both multiple and a completely different field from the "image" param used by POST and DELETE for space images. --- app/controllers/space_images_controller.rb | 4 ++-- app/views/spaces/show/_image_header_upload_button.html.erb | 4 ++-- spec/requests/space_images_spec.rb | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/controllers/space_images_controller.rb b/app/controllers/space_images_controller.rb index 7ccf4968..4936d2f4 100644 --- a/app/controllers/space_images_controller.rb +++ b/app/controllers/space_images_controller.rb @@ -19,8 +19,8 @@ def destroy end def upload_image - images = params["image"].compact_blank - return upload_failed if images.empty? + images = params["images_to_upload"]&.compact_blank + return upload_failed if images.blank? # Attempt to upload each image images.each do |image| diff --git a/app/views/spaces/show/_image_header_upload_button.html.erb b/app/views/spaces/show/_image_header_upload_button.html.erb index 20637550..decd8715 100644 --- a/app/views/spaces/show/_image_header_upload_button.html.erb +++ b/app/views/spaces/show/_image_header_upload_button.html.erb @@ -1,7 +1,7 @@ <%= form_with url: spaces_upload_image_path do |f| %> <%= f.hidden_field :id, value: @space.id %> - <%= f.file_field :image, accept: 'image/png,image/gif,image/jpeg', multiple: true, class: "hidden", onchange: "this.form.submit()" %> - <%= f.label :image, role: "button" do %> + <%= f.file_field :images_to_upload, accept: 'image/png,image/gif,image/jpeg', multiple: true, class: "hidden", onchange: "this.form.submit()" %> + <%= f.label :images_to_upload, role: "button" do %>

<%= t('images.upload_image') %> <%= inline_svg_tag 'camera', class: "text-gray-700", alt: t('images.upload'), title: t('images.upload') %> diff --git a/spec/requests/space_images_spec.rb b/spec/requests/space_images_spec.rb index 6ac19c9a..4135a59a 100644 --- a/spec/requests/space_images_spec.rb +++ b/spec/requests/space_images_spec.rb @@ -25,7 +25,7 @@ it "uploads an image and redirects" do expect do # Rails for some reason adds an empty "" to the start of the array when allowing multiple uploads - post spaces_upload_image_path(id: space.id), params: { image: ["", image_file] } + post spaces_upload_image_path(id: space.id), params: { images_to_upload: ["", image_file] } end.to change(space.images, :count).by(1) expect(response).to redirect_to(space_path(space.id)) follow_redirect!