From 4628b99445af395706353ebd4bd2e2073dc2f4b2 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Thu, 8 Mar 2018 12:25:07 +0000 Subject: [PATCH] Refactor to move preview logic to API * Moved the notifications code to go to admin to get the the template preview document rather than go to template preview. This will remove the logic from admin and place it in api so it is easier to expand on later when there are precompiled PDFs * Added some error handling if API returns an API error. Caught the error and displayed an error PNG so it is obvious something failed. Currently it displayed a thumbnail of a png over the top of the loading page, and therefore it wasn't obvious of the state. --- app/assets/images/preview_error.png | Bin 0 -> 16880 bytes app/main/views/notifications.py | 37 +++++++++-------- app/notify_client/notification_api_client.py | 11 +++++ tests/app/main/views/test_notifications.py | 40 ++++++++++++++++--- 4 files changed, 66 insertions(+), 22 deletions(-) create mode 100644 app/assets/images/preview_error.png diff --git a/app/assets/images/preview_error.png b/app/assets/images/preview_error.png new file mode 100644 index 0000000000000000000000000000000000000000..fe8b448cbc304822226228451225580f01636637 GIT binary patch literal 16880 zcmeHu2~?9=wr+4JN2CB11q2IA6cG>y<^VQPR0I@+fJ{nc3=n1lVN$yUWwK;aMioJk zAs{l3mJEf+Bm`uP5+INOk$Fz;{^{!Ke*JE})pcLry8YJEwOod9{_~%+&)(nn?R~a) zFX?D(Q0ot)P#c^!e*@nr=<)A>fBbx|7`f|R_uB95O0c$bu-fnL z?QFH*ifCtzLJ_+S!Wy^(k8RgrJ-Wr)EAG+eqW+Xm=$Ps%>Xuh-;^FWy`em!V3N?D3 zyZN{!4F|eIaeQ3do6U0j4(M1-ZpVwiKPotR;AT@?*eQwV?{1GuoZ(CkVrqJ-fH`E4-^j>H zACp-dj{Kx_$&P5{V1%)Av~qHTIniPY#}xkIx5j_>TUpr?C&grt4axknBQTK6uNFd< z`QQKN>q~x@jlY`ftGRxc1%B7yuio|5T))c#ziaSU@A`ksTwDL9v8~=JC}CW1dm*ePm2&hnCTwTW=ABl2 zkAkMzt8WObrqA1jH>k*bnlxf&dMu1J)AOjLyw+y7fV4R8>EUQt(7DG2DVyNmOW$2W zp#o3FZM+~6zOW93lB<||eU=_$?KJKM-_Ksw*KZqaj;=5w`JAEGo`}|qTLHnFBYWl6 zv$QmEdwct*Q6DUyxo7|RoQ8%*WOA^r$9M49oNe#U{QP`v|Er8JhDg+%J9muAeLOO+ z<{vR6oA=!yfS9g|H|WBDNY{Tp(p&5nX@|%29nLV!y0)Vat(rl}NYQ_;L$SlN&ZLKJK2*CNg(BSb z$k%kF?cE6P$&tv)P`2;%*k=jr3QPYWBK5A<-YSK%<}9n``-XV@8419LZLo< z3-VrFa2>4SoW^6Yklp5;a8}%Z)L>rT)Rav*dne7g9R9R!V_Y&Xz6z z%R;WTIcIOJir;?;^|w!OlK=fX36#n8dcc3Us-v2F=vv$K8C1?`crZtHQ4IBQ|F4cE z6#up?P+l8c>~Ej`XlMHu+wI>kn*PZR28Lz1&!7SuV*vr=3J{mhS>!_wu&@MmKS7qEAY?{LlYQ^lS>v)1m$?%Xe ze)YX|bJD&<e*!#8!mET zE4)7R^zCnIb|{avv}$yHI8_|=qSve&8tKU&3_ z4ZJ_m912`lr@@2X36r_Bo)M^d7cEKSPhP;xjJ6%R!y_spVCsB2)3o?iNySh@Y+Y)s zxgf1JEF{shTf%34VjqcB?v+5AD4vW6SV^go^&Qo+xVN;l^iaXYa%D&+;O^>NkId5C zxKNO&Ke5+~>NoB$)vUVgkl@*ECu><9JVMF9hVGFSlP>%6lQ_+!=LZ4vJ+P3;x1#Kv zE}X9F;*A8?cM`I$gFlb-m3nO#Ryfu_oitzFS;?LyuFiJGH*#R)F=ThOLWEGOhL{=~ zwdJmlj)i?sZJLviHz>@u>(h$qN%(nYIzs=w?rqwWpLqflJo~-rBVBnmGZTHvcesUi zA4>LhEn64}(H;&M{895I_2)v5@DC{ws)C4Ne%f-V`ra-{F|mQnye4FWMc_>F?@f%U zw>EAYfayc@brcWY*|AR$O_~dRq7hUp@jQVK*=a^|zVqjwiUuEOS#nnABP#rsYr~Z% z_K32-1mPI%>9TIa?`QjO=UjVpi*a1iFf*9CGA!>>&8vywuvc-Ps&2ac{9RDBN2gim zz@lOF{PX-~x1sXjV50&DW8dW-YItjkrY4L`!II5$JIzToHmcK)h>Jt9@tIzWQ$w=; zD+@2kC$v2qTE?3TJm@TCd4e7d>m zEVaTf#f#OS%c@we6{uXOd8q8=6zMI=F;|<*ji{L#YKYImcfYoy%unzHENJA_2?+`g zARg1+*;$v&o_TI)u_)0~?*=29OTIk3 zPK1<+!R%EbIu|c7GPE)ky$dmzQP`9>ElmDhey6#K*i&IJ$52}`+da}7{Lx%cQ1F68 z>cs?w+_8clCv6dOdb-`K3X7EifyUZnJ~JbHyhHaSvkwzUJw0(pIavh13^OtfOn$hp z!~;L+T^A;VJsT!yAoeuDrFO4&jPh8{+Y6W|eO+{Z{Y;NjX;!^biTmh>8>Y86@kk>F z7-^-0o^6L&^^=#b>^)(9(L)%~gdW8ceWkIb6D1j43truJ&tX73b;UYz6b@8k(?0i0 zk@Si7bp1%RP6{Pjd7>z#A`p>(`p?G>}lD3tGEeyK%e4iKDKk2z{viaH2J*^BP*QAu;yVRB}of>8Tcm{+t2BSFBqR~(zU}37G zH2Q36oDfhN^~xHT{pvepA+BxO^3lj*f7m7vW5HSKwCXhTlH$FLSaxb8B|>hm#=%Pf zO=iejRA`oot}ikilqt3cvm*-}b;nRQo-Ea@#_{zY zO^tnbM!@WfUzy8Tl-@f)?FQB5G5xR~MOQ~Ot(p=PttNVlhYqPNo}Vr6y|130o-U!x zDs%ImY1M21BR2rAy(_Gk&9c9UV6k>g)`km1)4l*USJawgE&RPDQr8UJ=S3}ti0mmO zECR%v4mTy5#7LW6^`9@L8gQhU492@QT{Sba`K-eJt9?buaOxpAwWX01Eld;yUA9tI zzWO6T0wluk2ET%@Vi5kddUI=|)#O0U5cty3w?RBIyqA||M)le3`ATeT56}cW-t0Hw z)|eD9_A!?-?=<-HkBuUn53v;&GWNe1~~2 zdw#qgblhvA*ScAMOH}aTl(S(+B(d2Ja?J~qv<%Ybj{XI3HDJBi?=G{3a2!KLrhG=vW68{`&mfTvy!*Pu z6mYfy(6mQbQ-ZTS%CVp;?P9!KR@YJ!9HjlxGWHUaGOD)nnJ1$`70|jdh6LChKbCG! zuD$soMKfYI?(r*|w1R?ym05B?mJWFXz!=aiInyZjY1b=@#dp}33 zwCJ(4QNXmtGVrK*S&{pAcYZrST6cH19%SVXGUu%bI1>^zPzp<0SEG1Xy%gDmnHpY{ z-2d^BP10V!r5U?vZ&l7>vwSJ`?JKK>fesTwSa5J&vubIL*4!M>Z^cwJ8H1@#=B&^k zDtXxFd4IEg&$rJ(VklYQaR?~_ejX4LQ(Ya^)g+sj$2GbEcc1CZHtQ*GdbfDcjoy@% zbfD#9C1<&^4!pyXy|O5kU6`FM-K54@v2H5_;8U3Ig)5waxgG*xawV`<$%C=8)6`8n zA%MMzkmPQgsZ? zzm3yY8wiFqJ&odr!V8fwDR? z(n+>BdGcg664|?B(VUfOT)bm}o!qLX+R|kVCV@C`Hy2pvB&m!vKQUR{>?bWIHc>?rOz+ zIhkS=p*Z}I!K&n}vT&^Od8erbFxJzwH0$x2?vqyUq8N<=a71kbD!{uMp@OH5i1^L4 zdSrH#?vVs2l_TXc@o- z3A>)dWGap>-aV9L%eMzFbnKcsV!*~UvT5=0B=6xz4^_NfRaU;drHleIX0!W?-RKIR z|I9z|Kuh|rkWlK7vS@jArTs-UDY}EBMh9y|UTn#iLdd zsY(?cY$vgfGOCp0`$8}HLW|yADRD%oP-bG0u@BB!pT($i2=?x+vi$JNpY;^~FpRJC2oB;F- zxsRd<@zc!(B=_OP>vTjI@rNsqzt)cjk~Uwr(daqE@510AwcO!Iin?0V0l9u> zdwfcoMWz2`ELK1(=IC9lh2N|;I>4lPwV<#(i?VoU``!-|9&^l2ne0mT5@JhB5S5b| z7OO}6XEQ8njBOHkv3%*uG%9ZH6O7K>f=n`KaV zd1V9GD-4C6B~ynH?c+UHy{yWz$~(dALKcn_V|mTtfGwm%oUQK}vfq54JVfH&NOA3Z zOmd}a#bg)(pyougihiGRs~)l7RQTMT8&<-nPMvZY%dM-8#+vV)?RM2-;d^;B$~@IAlR}h#yQ1)=xs# zq==wp9kz0bVomZZp!zQxcJ?gX&nb`&Fr8#taFkRL9S(`d%QgmsF;LF(BMAF`c_Vw{ z=lcpnQTkU+J*$IxU0P0Ws;R!qPb*<&hur2-Rk$EQd0Q%e@nKFsDYJv_oTGQ4beP$m z-sTf_-O@0gI)z`o^y(d7j5?aL)jFHo2_+YTz)@O z1SCX)Ta%(olOnwugcPFbgH4=@4^EV_Dva{&4NQF}Yh2)<>MBj4v`OBf21m+Jv6(-GlaVAA@HpVH6;_S4sh(%q!P2r<<#L}#lH`X6vi6Tatp z*151}8Fu=TvX}&7mceq|_f5Vkk~8&fYay=U@l5esc#|UM>nkATkeSMqvcM}{2JRlJ zl?hmEAPzsWc^Pyv-(91^t}QLUWWpxNTU@MT>=a04bhmIF5*vV{;Z2Fkb)uYwIt6-y z8<3z@S-X6CoDt)Y~DMfXJj zgxb|aa#me8jS#SG64It6L21Tl9V#ibMzWR(%Hnh~rN)Pxt&)^Mga7f{$RfUsh0+Jm@L2NUF{I9+p79esx7$O>6+-%?E>- z8SBbxj7;mStGc-!H#5@8%j*L5ntY;BE3Cgq?ya38*veYiz2!6{l{T*z7Mx1Q4biM8 zshvyJ{JPaF(971aNf*e>H8Issf<#K1B-0EAa1f{zN!P6Lw0NhN?EZ!OkMEh6!zDvt z1qiu}o5!#J^k=zb5-wfXe=aY1HM8bE;vA(iUs~c2zmU?Y1j50ARd{(R^N)cZM|guo zd~7k8m1fS0?yT5${PKiXr4X(zJnZ`0JEykP+>6vvk^lx{$y0sZFc@L4UYAy=T6u=y zg7$g7NIKw-Y|5v+>@cwImRyLqGZ6Tp`;_HGd`6zw+S&%|T~LAS&;_(rzK@O`2~r?G z7f83{7D;iSPG<+_4umU+A$6$CP9;g~O57>DVS@8dTN>q7K=-g07}!cdUcB6hN%sLF zRFY~xy}6A8c%y8_vp$>cLPYKX%SQ93k-DmiyQQwS4^Bck0bK#wL$f+g^7br?T3Jui z6!mj=XS7R*fEFS`mTU<*uH*3z1jiw*D-l`dc0~VNj+VA*`8)0i@h$WLswno%#mA3VySucT>WZUYZHfnK;lS zYtZTis$_x1VY7-rb65hxO)@gbwSbrW+XSRT!H@EN4!3lNt1gWwfG}MX^Cy}V4rG_l zbt^!qB070;FjBX6b)G89i{BCX(pRJJqi2t!fG&FWN*O4gP#0QlO(56$xUrhWF&nj> zZ1lN9iSBy7;BO%{Ap&)!)YXZH$FBcr3^EW|(%cwQYNL%lPY##G)mBGI=v(%ec~3$f zoL`g#yHn|B&lO^25|QMH(5~*#AlKu-kH`n)M6KUH2PQyr7`?i2e!GZ@LUsw`<*e$B(Vq`Qgv$7S-nLdA zmGPepLGW+@kS*fP?M-!Xj>hGJ9UyZDUS?$5yX*s^xVMF0yt=buE+5geV`bA#h}E+? z6%Yzin0|kPh7e1y^IPu89x7W7oP&S;rX6dDT!OvP{^IhQyO~q~(;(%)*F++=I&PbE zs0doge>$Fusg6TS3;DFJyC(TbXG$F}NImcG=U*EkYGI~HQ?9QB(34Wt9nzUHEA@1A zQvtUaB2kykM}ac(=wsB%fMO_fHb6N$P6}HbxuQrW@d$6(ARSH(p3JM+i*3 zfm`t83sVn@fmC>9zH$*2;>QZ2A1`+Y=8nQ=7pFl66cyn=lR}|8HX;>9#;XYQoKePf z7E@=cu-8>9qyfD=mPhdx*N$r+n;dO>9*YoYJ;}9-Yf}_JLk#4mnM-lq8Iy^FpnWH% zfC(zQNbW!@F-Xm(F(#|`yuVp89cn7|y8e2IY<1{MRQ8faJT1g4hcJflj>jW}_OhBf zyAubsjHkOQpg`0B2p=DYc=1r^i6&c5`GwjeECMDfo={QTMT4MonVp0}N?v25qCZtA+isdEYDjPP_zi@ zt+e0+;lazSO10h6#wK#mxhdd|^a7af;Zgab{ytt_#L*$2!+RMdDM#-vvalO;eEGaJyC>onKL1M?CdPp1`Zj2Xz?24i_Z4UT1nJVKa zffVe;gKF}JK!>C!#ZE(5R>Q_|AD*3`1d_!xC!ukbt8@5S_7Uc?cebgy4-~0$p_YqW zbyz~x&tX1#1hnKX2#8iFBpS*XufVNAHKYY15VQ8Pz9|M7hBX%KsaPT;#JHh%(X#F% zEgxZfDUcF5s&b%sVF{Hr-qtB<=y z8gAYh*jU|t^T8R3gHUwdg=8Ta*(FDiigMvtF_eTp5P`yINCrPU$V6%jVeV(}c z_IlG3$FH)wfpE7RL(6h)Ey#>#h!Mp-sl&LpDEw;k*}+HYtB&5h;m(CJ`Tn;$ z3H}w`SA;C2fd}>T#m7}98A#&->i(mD0$-q!s*)cSD1Z8KRoZ9&L(oZf>py$SK}Gt( zI-zr5*}r}I+PU)^O>_4(p;=5_lmD+>b^9Zb_8Rw-H`j4)6e^+AH}N0-qFa%_4mEP^ z*Vd^2)R-%g4cphF+>gP7gU8>ZQ2Q>z0SsOzsG6XEb)_liBH6$D@sH?+v(=DDO$lmw z`5w1EP+kj`ge*Q#4$3XB7m1`f%S?RI+{6}RQbkN{M|Vj_k`rqi%6JQ?lPk2JiFrS+ zM|B~?-3PxZe~7w<|Nat6uBv1Q7rt3#+_-M({k$Q08%j?6uN&;nDIT-?Xj;1k#e{~m z0#p;0>zWpHFm~t2%B{#7syMvg%zF}b10wi=>&M+FlLI%`wcL6CV++yYQ5`%3{frKl zLXskKwFkpGy;M=ejK8)N=InYgCYwv&5IFW*5&V~pq5kh0ma1y?m3&Y+#fUcu>1u28 zpya&%&!_(H(w6yO0Cc%S(8-B%xQcWUCLq`xsQp*}_}|Ug(rWr>+Y7$K>Uf7^D8er6 z@Vb{9wzdcgZAVp^==|bUe|gH`F#RQ1FHD_FF3QaSnp)vD_7Cq;{V$l~9~j~5n*WN3 z|H!KUf6JM#*7!Xs#UiJ>5GO2Qxj*nJ#na;2L_kJy^9y-(?vM@Lfg3AcvkrQGsv3Ke4`qctoE%4O>UoG&}0)LDJLOq1* z-oH2PoJ}SJEBS8pBK1m5_0J1L(G>i>K`DO9^KIY9RGsk$j#>~>wU3zY&&;=Gp--G> zrcaBSa>KXwbNzW85tkR3CI0uK-G)udGxLX4n!Qy-rhUqXOQhW-;QyS7mR5RDsOT03Lx;O#lD@ literal 0 HcmV?d00001 diff --git a/app/main/views/notifications.py b/app/main/views/notifications.py index 365ffb25b..350fdd92b 100644 --- a/app/main/views/notifications.py +++ b/app/main/views/notifications.py @@ -1,4 +1,6 @@ # -*- coding: utf-8 -*- +import base64 +import os from datetime import datetime from flask import ( @@ -11,6 +13,7 @@ from flask import ( url_for, ) from flask_login import login_required +from notifications_python_client.errors import APIError from app import ( current_service, @@ -19,7 +22,7 @@ from app import ( notification_api_client, ) from app.main import main -from app.template_previews import TemplatePreview, get_page_count_for_letter +from app.template_previews import get_page_count_for_letter from app.utils import ( DELIVERED_STATUSES, FAILURE_STATUSES, @@ -83,6 +86,12 @@ def view_notification(service_id, notification_id): ) +def get_preview_error_image(): + path = os.path.join(os.path.dirname(__file__), "..", "..", "assets", "images", "preview_error.png") + with open(path, "rb") as file: + return file.read() + + @main.route("/services//notification/.") @login_required @user_has_permissions('view_activity') @@ -91,23 +100,19 @@ def view_letter_notification_as_preview(service_id, notification_id, filetype): if filetype not in ('pdf', 'png'): abort(404) - notification = notification_api_client.get_notification(service_id, notification_id) - notification['template'].update({'reply_to_text': notification['reply_to_text']}) + try: + preview = notification_api_client.get_notification_letter_preview( + service_id, + notification_id, + filetype, + page=request.args.get('page') + ) - template = get_template( - notification['template'], - current_service, - letter_preview_url=url_for( - '.view_letter_notification_as_preview', - service_id=service_id, - notification_id=notification_id, - filetype='png', - ), - ) + display_file = base64.b64decode(preview['content']) + except APIError: + display_file = get_preview_error_image() - template.values = notification['personalisation'] - - return TemplatePreview.from_utils_template(template, filetype, page=request.args.get('page')) + return display_file @main.route("/services//notification/.json") diff --git a/app/notify_client/notification_api_client.py b/app/notify_client/notification_api_client.py index 9e80af511..6f903e835 100644 --- a/app/notify_client/notification_api_client.py +++ b/app/notify_client/notification_api_client.py @@ -73,3 +73,14 @@ class NotificationApiClient(NotifyAdminAPIClient): if notification['notification_type'] == 'letter' and notification['status'] in ('created', 'sending'): notification['status'] = 'accepted' return notifications + + def get_notification_letter_preview(self, service_id, notification_id, file_type, page=None): + + get_url = '/service/{}/template/preview/{}/{}{}'.format( + service_id, + notification_id, + file_type, + '?page={}'.format(page) if page else '' + ) + + return self.get(url=get_url) diff --git a/tests/app/main/views/test_notifications.py b/tests/app/main/views/test_notifications.py index 958c2827d..0f89d2de4 100644 --- a/tests/app/main/views/test_notifications.py +++ b/tests/app/main/views/test_notifications.py @@ -1,9 +1,11 @@ +import base64 from functools import partial +from unittest.mock import mock_open import pytest from flask import url_for from freezegun import freeze_time -from notifications_utils.template import LetterImageTemplate +from notifications_python_client.errors import APIError from tests.conftest import ( SERVICE_ONE_ID, mock_get_notification, @@ -163,9 +165,11 @@ def test_should_show_image_of_letter_notification( mock_get_notification(mocker, fake_uuid, template_type='letter') - mocked_preview = mocker.patch( - 'app.main.views.templates.TemplatePreview.from_utils_template', - return_value='foo' + mocker.patch( + 'app.notify_client.notification_api_client.NotificationApiClient.get', + return_value={ + 'content': base64.b64encode(b'foo').decode('utf-8') + } ) response = logged_in_client.get(url_for( @@ -177,8 +181,32 @@ def test_should_show_image_of_letter_notification( assert response.status_code == 200 assert response.get_data(as_text=True) == 'foo' - assert isinstance(mocked_preview.call_args[0][0], LetterImageTemplate) - assert mocked_preview.call_args[0][1] == filetype + + +def test_should_show_preview_error_image_letter_notification_on_preview_error( + logged_in_client, + fake_uuid, + mocker, +): + + mock_get_notification(mocker, fake_uuid, template_type='letter') + + mocker.patch( + 'app.notify_client.notification_api_client.NotificationApiClient.get', + side_effect=APIError + ) + + mocker.patch("builtins.open", mock_open(read_data="preview error image")) + + response = logged_in_client.get(url_for( + 'main.view_letter_notification_as_preview', + service_id=SERVICE_ONE_ID, + notification_id=fake_uuid, + filetype='png' + )) + + assert response.status_code == 200 + assert response.get_data(as_text=True) == 'preview error image' def test_should_404_for_unknown_extension(