IMAGEDAM-1674: Notification banner displays messages about send to PhotoSales success/failure

[Conalb97]

What does this change?

Note: this PR should be reviewed after https://github.com/guardian/grid/pull/4267 as it builds on the changes included in that PR

The relevant files for this PR are:

• kahuna/public/js/components/gr-notifications-banner/gr-notifications-banner.tsx
• kahuna/public/js/search/results.js
• kahuna/public/js/util/constants/sendToCapture-config.js

Once a user has confirmed that they want to send images to photosales, they will see a banner notification indicating that the image(s) has been sent successfully (if there were no issues), a warning (if all images sent already exist in Capture) or that there was an error in sending the image (if an error occurs).

The Photosales functionality is BBC specific, hence it is disabled by default via the showSendToPhotoSales feature flag.

How should a reviewer test this change?

As a user with elevated permissions and with the showSendToPhotoSales flag set to true:

Success banner

• Select a number of image thumbnails from the main grid view, ensure none of these images have already been sent to Photosales
• Click the 'Send to Photo Sales Button' from the menu that appears
• A confirmation dialog modal will pop up
• Click 'Yes, send'
• A green notification banner will pop up

Warning banner

• Select a number of image thumbnails from the main grid view, ensure that all of these images have been sent to Photosales
• Click the 'Send to Photo Sales Button' from the menu that appears
• A confirmation dialog will pop up
• Click 'Yes, send'
• An orange warning banner will pop up

Error banner

• Take a new line on 424 of kahuna/public/js/search/results.js and enter throw new Error(‘test’);
• Refresh your browser
• Select a number of image thumbnails from the main grid view, ensure none of these images have already been sent to Photosales
• Click the ‘Send to Photo Sales Button’ from the menu that appears
• A red notification banner will pop up

How can success be measured?

Success banner

• Confirm that the success banner has the following appearance

Warning

• Confirm that the warning banner has the following appearance

Error

• Confirm that the error banner has the following appearance

Who should look at this?

Tested? Documented?

• [x] locally by committer
• [ ] locally by Guardian reviewer
• [ ] on the Guardian's TEST environment
• [ ] relevant documentation added or amended (if needed)

[AndyKilmory]

Specific messaging changes look okay - minor comments on announceId used in each case

[AndyKilmory]

All looks good now from notification perspective

[Conalb97]

I am unable to test this locally as the build fails due to validImages having been declared twice in kahuna/public/js/search/results.js

This is passing now - looks like there were some issues with the branch conflicting with other related PR's that had been merged in first.

[Conalb97]

I was initially confused testing this as I could not get the banner to show when I had showSendToPhotoSales=true in config.

I figured out that in order to have the banner appear at all, you also need at least one announcement defined in config as the component is wrapped in ng-if="notifctrl.hasNotifications" and will not mount otherwise.

While I assume the BBC has other announcements defined in config so this issue doesn't come up, it highlights an issue where this code will not work if there are no announcements defined in config.

Perhaps, rather than defining some announcements in config and hard-coding others in code, all announcements should be defined in the same way (ie in config) and can be passed as props into the React component which can then reference them by their announceId.

Happy to hear thoughts

Hi Rebecca, I've pushed a change that removes the conditional on the notification banner. @AndyKilmory suggested this change as it simplifies the control and ensures that it will always be intialised, which allows us to maintain the current patterns.

[Conalb97]

Thanks for the approval @rebecca-thompson, really appreciate it! Would someone at the Guardian be able to merge? I don't have access to do so.

[prout-bot]

Seen on image-loader, leases, auth, metadata-editor (created by @Conalb97 and merged by @paperboyo 7 minutes and 41 seconds ago) Please check your changes!

[prout-bot]

Seen on collections (created by @Conalb97 and merged by @paperboyo 7 minutes and 46 seconds ago) Please check your changes!

[prout-bot]

Seen on usage, kahuna (created by @Conalb97 and merged by @paperboyo 7 minutes and 49 seconds ago) Please check your changes!

[prout-bot]

Seen on thrall (created by @Conalb97 and merged by @paperboyo 7 minutes and 55 seconds ago) Please check your changes!

[prout-bot]

Seen on thrall, cropper (created by @Conalb97 and merged by @paperboyo 7 minutes and 55 seconds ago) Please check your changes!

[prout-bot]

Seen on media-api (created by @Conalb97 and merged by @paperboyo 8 minutes and 5 seconds ago) Please check your changes!