guardian / grid

The Guardian’s image management system
https://www.theguardian.com/info/developer-blog/2015/aug/12/open-sourcing-grid-image-service
Apache License 2.0
1.44k stars 120 forks source link

1443 add keywords uploadpage #4139

Closed AndyKilmory closed 11 months ago

AndyKilmory commented 12 months ago

Purpose of change is to add 'Keywords' field to the Upload page so that when a user uploads an image they are able to add in 'Keywords' in the same way that they can add in 'Labels' at the time of uploading.

The control has been designed to mimic the look and feel of the pre-existing Add Labels control whilst the styling of the pills showing the added keywords matches that seen on the main metadata info panel.

Screenshot 2023-09-12 at 12 15 24

Testing should verify that keywords can be added and removed in similar fashion to labels and that all other metadata remains intact.

Tested? Documented?

paperboyo commented 12 months ago

Hi, @AndyKilmory! This looks very useful. Most of us are at a sports day, but just some initial comments (first point looks serious):

  1. Batch application is confused between Labels and Keywords: sometimes adding/removing one, adds/removes the other:

  2. Hovering over a keyword to launch a search, actually launches a Label search instead: search_links

Will have a play some more. I have some minor issues with how it works (no way of committing text field with Enter, or at least Cmd+Enter; some minor rendering/CSS issues), but they also apply to labels so are preexisting.

AndyKilmory commented 12 months ago

Hi Mateusz, Thanks for the comments – I’ll take a look – you’re UI looks a little different from mine – so I think my configuration isn’t quite the same re. batch application – so will need to figure out how to get the same functionality available in my UI. I thought I had supressed the search in keywords as it seemed to be unhelpful given the range of keywords in the system – will take a look.

Regards, Andy

Andy Downing (he/him) Senior Software Engineer (Images) Business Systems and Applications Broadcast & End-User Technology (BEUT) BBC Technology Group

From: Mateusz @.> Reply to: guardian/grid @.> Date: Tuesday, 12 September 2023 at 15:27 To: guardian/grid @.> Cc: Andrew Downing @.>, Mention @.***> Subject: Re: [guardian/grid] 1443 add keywords uploadpage (PR #4139)

Hi, @AndyKilmoryhttps://github.com/AndyKilmory! This looks very useful. Most of us are at a sports day, but just some initial comments (first point looks serious):

  1. Batch application is confused between Labels and Keywords: sometimes adding/removing one, adds/removes the other: [https://user-images.githubusercontent.com/6032869/267360514-4191ba46-4031-4541-b79d-9c73df7be757.gif]https://user-images.githubusercontent.com/6032869/267360514-4191ba46-4031-4541-b79d-9c73df7be757.gif
  2. Hovering over a keyword to launch a search, actually launches a Label search instead: [search_links]https://user-images.githubusercontent.com/6032869/267360772-c6060439-2602-4384-9829-2629a411c706.gif

Will have a play some more. I have some minor issues with how it works (no way of committing text field with Enter, or at least Cmd+Enter; some minor rendering/CSS issues), but they also apply to labels so are preexisting.

— Reply to this email directly, view it on GitHubhttps://github.com/guardian/grid/pull/4139#issuecomment-1715835186, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A6UE4XUKPJA6QB72XRP4C6TX2BWNXANCNFSM6AAAAAA4UWQIQU. You are receiving this because you were mentioned.Message ID: @.***>

paperboyo commented 12 months ago

you’re UI looks a little different from mine – so I think my configuration isn’t quite the same re. batch application

I think it’s the fact that for Batch UI to show, you need to upload more than one pic (and keep the session, eg. reloading the page will break the batch).

AndyKilmory commented 12 months ago

Thanks – will try to replicate locally and work on a fix

Andy Downing (he/him) Senior Software Engineer (Images) Business Systems and Applications Broadcast & End-User Technology (BEUT) BBC Technology Group

From: Mateusz @.> Reply to: guardian/grid @.> Date: Tuesday, 12 September 2023 at 15:40 To: guardian/grid @.> Cc: Andrew Downing @.>, Mention @.***> Subject: Re: [guardian/grid] 1443 add keywords uploadpage (PR #4139)

you’re UI looks a little different from mine – so I think my configuration isn’t quite the same re. batch application

I think it’s the fact that for Batch UI to show, you need to upload more than one pic (and keep the session, eg. reloading the page will break the batch).

— Reply to this email directly, view it on GitHubhttps://github.com/guardian/grid/pull/4139#issuecomment-1715858401, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A6UE4XW32O6LCKPTSOXIJKTX2BX4RANCNFSM6AAAAAA4UWQIQU. You are receiving this because you were mentioned.Message ID: @.***>

AndyKilmory commented 11 months ago

Issues 1. and 2. raised by Guardian review now addressed.

Have added task to BBC Scrum board to add event to Datalist control to respond to 'Enter' key press to trigger save event.

AndyKilmory commented 11 months ago

Hi Mateusz, Have made amendments to correct the issue you raised.

Have spoken with the team and added a task to our Sprint board to look at adding an event to trigger on the ‘Enter’ key press that will save any updates to the Keywords and Labels.

Thanks, Andy

Andy Downing (he/him) Senior Software Engineer (Images) Business Systems and Applications Broadcast & End-User Technology (BEUT) BBC Technology Group

From: Andrew Downing @.> Date: Tuesday, 12 September 2023 at 15:41 To: guardian/grid @.>, guardian/grid @.> Cc: Mention @.> Subject: Re: [guardian/grid] 1443 add keywords uploadpage (PR #4139)

Thanks – will try to replicate locally and work on a fix

Andy Downing (he/him) Senior Software Engineer (Images) Business Systems and Applications Broadcast & End-User Technology (BEUT) BBC Technology Group

From: Mateusz @.> Reply to: guardian/grid @.> Date: Tuesday, 12 September 2023 at 15:40 To: guardian/grid @.> Cc: Andrew Downing @.>, Mention @.***> Subject: Re: [guardian/grid] 1443 add keywords uploadpage (PR #4139)

you’re UI looks a little different from mine – so I think my configuration isn’t quite the same re. batch application

I think it’s the fact that for Batch UI to show, you need to upload more than one pic (and keep the session, eg. reloading the page will break the batch).

— Reply to this email directly, view it on GitHubhttps://github.com/guardian/grid/pull/4139#issuecomment-1715858401, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A6UE4XW32O6LCKPTSOXIJKTX2BX4RANCNFSM6AAAAAA4UWQIQU. You are receiving this because you were mentioned.Message ID: @.***>


http://www.bbc.co.uk This e-mail (and any attachments) is confidential and may contain personal views which are not the views of the BBC unless specifically stated. If you have received it in error, please delete it from your system. Do not use, copy or disclose the information in any way nor act in reliance on it and notify the sender immediately. Please note that the BBC monitors e-mails sent or received. Further communication will signify your consent to this.


paperboyo commented 11 months ago

Thanks for the fixes! Testing some more, I can see the following issue.

  1. Removing keywords doesn’t work through Batch apply: Steps:

    1. Upload more than one image (so Batch functionality is there)
    2. Apply more than one keyword to an image
    3. Click Apply keywords to all arrows
    4. Remove one of the keywords from an image
    5. Click Apply keywords to all arrows

    Expected result: The removed keyword should be removed from all images (works if one deletes all keywords, but not when one deletes just one) Actual result: Batch apply doesn’t remove deleted keyword from other images in the batch

Sadly, it looks like it affects Labels too, so it’s pre-existing! (?) Unsure, then, if it’ś fair to expect it to be fixed as part of this PR…

AndyKilmory commented 11 months ago

Hi Mateusz, I was getting the batch addition to work and the batch removal of all entries – but I couldn’t get the removal of a single label or keyword to propagate via batch – so wasn’t sure if this was expected. I’ll see if I can fix it for both controls – now I know what the expected behaviour is.

Regards, Andy

Andy Downing (he/him) Senior Software Engineer (Images) Business Systems and Applications Broadcast & End-User Technology (BEUT) BBC Technology Group

From: Mateusz @.> Reply to: guardian/grid @.> Date: Friday, 15 September 2023 at 12:36 To: guardian/grid @.> Cc: Andrew Downing @.>, Mention @.***> Subject: Re: [guardian/grid] 1443 add keywords uploadpage (PR #4139)

Thanks for the fixes! Testing some more, I can see the following issue.

  1. Removing keywords doesn’t work through Batch apply: Steps:
    • Upload more than one image (so Batch functionality is there)
    • Apply more than one keyword to an image
    • Click Apply keywords to all arrows
    • Remove one of the keywords from an image
    • Click Apply keywords to all arrows

Expected result: The removed keyword should be removed from all images (works if one deletes all keywords, but not when one deletes just one) Actual result: Batch apply doesn’t remove deleted keyword from other images in the batch

Sadly, it looks like it affects Labels too, so it’s pre-existing! (?) Unsure, then, if it’ś fair to expect it to be fixed as part of this PR…

— Reply to this email directly, view it on GitHubhttps://github.com/guardian/grid/pull/4139#issuecomment-1721129543, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A6UE4XSIR5MYWDHD7YCTXJDX2Q4TPANCNFSM6AAAAAA4UWQIQU. You are receiving this because you were mentioned.Message ID: @.***>

paperboyo commented 11 months ago

I’ll see if I can fix it for both controls – now I know what the expected behaviour is

Thank you so much, @AndyKilmory!

paperboyo commented 11 months ago

Hi, @AndyKilmory, testing more on our TEST env, the (preexisting) issue above is now fixed, thank you so much. There is one remaining part of it that is still not working as expected: again, this is preexisting, but very much related. It wasn’t so obvious with Labels, because we don’t read Labes from image metadata, so it would manifest itself only while reuploading images that have Labels already, but it’s more important for Keywords as those we read from images.

Basically: Batch apply arrow button should copy Labels/Keywords from the source image exactly to all other images. After your fix it removes items correctly, but not for previously existing Labels/Keywords:

  1. Upload at least two images, one with preexisting Keyword/Label (either because it was reuploaded, or for keywords: because there is one in the image metadata). Image A has preexisting Keyword Z, Image B has no Keywords.
  2. Add Keyword Y to Image B.
  3. Click Batch apply button on Image B.

Expected result: Keywords from Image B are copied verbatim to Image A. Both images have only Keyword Y. Keyword Z should be removed from Image A.

Actual result: Keyword Z is not removed from Image A, so Image A now has two Keywords (Z & Y) instead of only Y.

Again, issue is preexisting, but I thought, if easy, worth fixing as part of this PR. But do let us know if you want this PR merged and we can open an issue to fix this later!

AndyKilmory commented 11 months ago

Hi Mateusz, Thanks for the update – the issue you highlight will be very easy to fix as the inclusion of pre-existing keywords/labels when batch updating is explicit in the code – I just followed the same pattern for keywords as was being executed for labels. It will be an easy change to replicate only the keywords/labels from the originating image across the batch upload images.

I’m on leave now until next Wednesday so it will be a few days before the changes get uploaded to github.

Thanks, Andy

Andy Downing (he/him) Senior Software Engineer (Images) Business Systems and Applications Broadcast & End-User Technology (BEUT) BBC Technology Group

From: Mateusz @.> Reply to: guardian/grid @.> Date: Wednesday, 20 September 2023 at 13:20 To: guardian/grid @.> Cc: Andrew Downing @.>, Mention @.***> Subject: Re: [guardian/grid] 1443 add keywords uploadpage (PR #4139)

Hi, @AndyKilmoryhttps://github.com/AndyKilmory, testing more on our TEST env, the (preexisting) issue above is now fixed, thank you so much. There is one remaining part of it that is still not working as expected: again, this is preexisting, but very much related. It wasn’t so obvious with Labels, because we don’t read Labes from image metadata, so it would manifest itself only while reuploading images that have Labels already, but it’s more important for Keywords as those we read from images.

Basically: Batch apply arrow button should copy Labels/Keywords from the source image exactly to all other images. After your fix it removes items correctly, but not for previously existing Labels/Keywords:

  1. Upload at least two images, one with preexisting Keyword/Label (either because it was reuploaded, or for keywords: because there is one in the image metadata). Image A has preexisting Keyword Z, Image B has no Keywords.
  2. Add Keyword Y to Image B.
  3. Click Batch apply button on Image B.

Expected result: Keywords from Image B are copied verbatim to Image A. Both images have only Keyword Y. Keyword Z should be removed from Image A.

Actual result: Keyword Z is not removed from Image A, so Image A now has two Keywords (Z & Y) instead of only Y.

Again, issue is preexisting, but I thought, if easy, worth fixing as part of this PR. But do let us know if you want that PR merged and we can open an issue to fix this later!

— Reply to this email directly, view it on GitHubhttps://github.com/guardian/grid/pull/4139#issuecomment-1727613104, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A6UE4XV46ZH7WYTV3DBI5GLX3LNRBANCNFSM6AAAAAA4UWQIQU. You are receiving this because you were mentioned.Message ID: @.***>

AndyKilmory commented 11 months ago

Hi Mateusz, We discussed this issue at sprint review and planning today. Would it be possible to approve and deploy the functionality as it stands (as it is existing behaviour) with the aim of reviewing and amending it under a future PR. There were some members of the team who wanted to retain the ‘apply as update’ functionality that currently exists whilst others preferred the overwrite as suggested – so the UX team would like to explore if it will be possible to provide the user with the option to follow either path.

There was also the desire to add in the ability to easily remove all keywords from an image without having to select each in turn to handle case where image metadata contains numerous silly keywords.

The hope was that this would be a ‘quick win’ piece of functionality that we could get to the users quite quickly.

Thanks, Andy

Andy Downing (he/him) Senior Software Engineer (Images) Business Systems and Applications Broadcast & End-User Technology (BEUT) BBC Technology Group

From: Mateusz @.> Reply to: guardian/grid @.> Date: Wednesday, 20 September 2023 at 13:20 To: guardian/grid @.> Cc: Andrew Downing @.>, Mention @.***> Subject: Re: [guardian/grid] 1443 add keywords uploadpage (PR #4139)

Hi, @AndyKilmoryhttps://github.com/AndyKilmory, testing more on our TEST env, the (preexisting) issue above is now fixed, thank you so much. There is one remaining part of it that is still not working as expected: again, this is preexisting, but very much related. It wasn’t so obvious with Labels, because we don’t read Labes from image metadata, so it would manifest itself only while reuploading images that have Labels already, but it’s more important for Keywords as those we read from images.

Basically: Batch apply arrow button should copy Labels/Keywords from the source image exactly to all other images. After your fix it removes items correctly, but not for previously existing Labels/Keywords:

  1. Upload at least two images, one with preexisting Keyword/Label (either because it was reuploaded, or for keywords: because there is one in the image metadata). Image A has preexisting Keyword Z, Image B has no Keywords.
  2. Add Keyword Y to Image B.
  3. Click Batch apply button on Image B.

Expected result: Keywords from Image B are copied verbatim to Image A. Both images have only Keyword Y. Keyword Z should be removed from Image A.

Actual result: Keyword Z is not removed from Image A, so Image A now has two Keywords (Z & Y) instead of only Y.

Again, issue is preexisting, but I thought, if easy, worth fixing as part of this PR. But do let us know if you want that PR merged and we can open an issue to fix this later!

— Reply to this email directly, view it on GitHubhttps://github.com/guardian/grid/pull/4139#issuecomment-1727613104, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A6UE4XV46ZH7WYTV3DBI5GLX3LNRBANCNFSM6AAAAAA4UWQIQU. You are receiving this because you were mentioned.Message ID: @.***>

paperboyo commented 11 months ago

Sure! Have let the team know to give it a re-review and merge as is (it’s already better as it was, thanks!).

some (...) who wanted to retain the ‘apply as update’ functionality that currently exists whilst others preferred the overwrite as suggested

The Batch apply buttons were always thought of as Synchronise commands: copying the state of the relevant fields as-is to all the images in the upload batch. It was a bug that this wasn’t working for arrays (adding Keywords only made it more obvious as Labels never arrive with metadata). So we will need to fix it to behave the same for preexisting Keywords (and Labels) too regardless of any improvements.

Now, being able to a) apply only specific members of an array to all other uploads b)remove all array members from an image in on go (to potentially Batch apply this later to all others) are both potentially useful. At least a) already exists in the Browser (on a search page) when editing arrays when multiple images are selected (little plus sign to the left of an array member). Not sure if adding them to the upload page wouldn’t be confusing, but it is a possibility. As to b), I think it’s a good idea (especially for keywords as these can be very messy), but any solution should also work in the Browser and consistently too.

In short, while I think it’s possible to improve things (always!), the fix that I suggested above (for Batch apply to work the same for preexisting as for new array members) isn’t related to any future improvements. Until we do, we have an inconsistent behaviour only for preexisting array members.

AndyKilmory commented 11 months ago

Hi Mateusz, Thanks for the feedback. I’ll pass your comments onto the team. We have a task in the next sprint to revisit the batch upload and we can look to remove the inconsistencies then.

Regards, Andy

Andy Downing (he/him) Senior Software Engineer (Images) Business Systems and Applications Broadcast & End-User Technology (BEUT) BBC Technology Group

From: Mateusz @.> Reply to: guardian/grid @.> Date: Monday, 2 October 2023 at 20:43 To: guardian/grid @.> Cc: Andrew Downing @.>, Mention @.***> Subject: Re: [guardian/grid] 1443 add keywords uploadpage (PR #4139)

Sure! Have let the team know to give it a re-review and merge as is (it’s already better as it was, thanks!).

some (...) who wanted to retain the ‘apply as update’ functionality that currently exists whilst others preferred the overwrite as suggested

The Batch apply buttons were always thought of as Synchronise commands: copying the state of the relevant fields as-is to all the images in the upload batch. It was a bug that this wasn’t working for arrays (adding Keywords only made it more obvious as Labels never arrive with metadata). So we will need to fix it to behave the same for preexisting Keywords (and Labels) too regardless of any improvements.

Now, being able to a) apply only specific members of an array to all other uploads b)remove all array members from an image in on go (to potentially Batch apply this later to all others) are both potentially useful. At least a) already exists in the Browser (on a search page) when editing arrays when multiple images are selected (little plus sign to the left of an array member). Not sure if adding them to the upload page wouldn’t be confusing, but it is a possibility. As to b), I think it’s a good idea (especially for keywords as these can be very messy), but any solution should also work in the Browser and consistently too.

In short, while I think it’s possible to improve things (always!), the fix that I suggested above (for Batch apply to work the same for preexisting as for new array members) isn’t related to any future improvements. Until we do, we have an inconsistent behaviour only for preexisting array members.

— Reply to this email directly, view it on GitHubhttps://github.com/guardian/grid/pull/4139#issuecomment-1743649516, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A6UE4XWMT3KVWNEHNP3MEJDX5MKPZAVCNFSM6AAAAAA4UWQIQWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONBTGY2DSNJRGY. You are receiving this because you were mentioned.Message ID: @.***>

AndyKilmory commented 11 months ago

Hi Tom, Thanks for the PR approval. Changes to main branch have been merged in and checks are currently re-running.

These changes have raised some queries about the behaviour of ‘Apply to all’ in relation to Labels and Keywords and we will be re-visiting this issue over the next few weeks and I’ll pick up your comments regarding using ‘const’ rather than ‘let’ when I come back to this then (hope that is okay). It was supposed to be a simple change to meet user requests which has grown a bit larger than planned.

Thanks, Andy

Andy Downing (he/him) Senior Software Engineer (Images) Business Systems and Applications Broadcast & End-User Technology (BEUT) BBC Technology Group

From: Tom Richards @.> Reply to: guardian/grid @.> Date: Monday, 9 October 2023 at 11:35 To: guardian/grid @.> Cc: Andrew Downing @.>, Mention @.***> Subject: Re: [guardian/grid] 1443 add keywords uploadpage (PR #4139)

@twrichards approved this pull request.

code LGTM (although I've left some suggestions for avoiding let where possible, although there's probably more that I didn't spot) - @paperboyohttps://github.com/paperboyo has tested functionality and I appreciate its been a long time since this PR was raised - thanks for the contribution(s).

Please could you rebase off (or merge in) latest main (we've been making changes to the CI GitHub Action, which this PR needs to sit on top of) then I can merge this in...


In kahuna/public/js/components/gr-add-keyword/gr-add-keyword.jshttps://github.com/guardian/grid/pull/4139#discussion_r1350126022:

+

+import '../../directives/gr-auto-focus';

+import {overwrite} from "../../util/constants/editOptions";

+

+export var addKeyword = angular.module('gr.addKeyword', [

+]);

+

+addKeyword.controller('GrAddKeywordCtrl', [

not re-assigned so needn't be a let ⬇️ Suggested change

it is mutated too, but that's angular's way 😢


In kahuna/public/js/components/gr-add-keyword/gr-add-keyword.jshttps://github.com/guardian/grid/pull/4139#discussion_r1350126971:

  • 'gr.image.service',

+]);

+

+addKeyword.controller('GrAddKeywordCtrl', [


In kahuna/public/js/components/gr-add-keyword/gr-add-keyword.jshttps://github.com/guardian/grid/pull/4139#discussion_r1350127637:

  • ctrl.descriptionOption = overwrite.key;

+


In kahuna/public/js/components/gr-add-keyword/gr-add-keyword.jshttps://github.com/guardian/grid/pull/4139#discussion_r1350128238:

  • let uptodateImage = tmpImages[0];

— Reply to this email directly, view it on GitHubhttps://github.com/guardian/grid/pull/4139#pullrequestreview-1664155518, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A6UE4XQLHABM47QCQMEEDVLX6PHQVAVCNFSM6AAAAAA4UWQIQWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTMNRUGE2TKNJRHA. You are receiving this because you were mentioned.Message ID: @.***>

twrichards commented 11 months ago

hi @AndyKilmory - sorry for the delay, but if you could please do one last rebase/merge of main into your branch, then CI should succeed and I can merge - thank you

AndyKilmory commented 11 months ago

Hi Tom, Will do

Thanks, Andy

Andy Downing (he/him) Senior Software Engineer (Images) Business Systems and Applications Broadcast & End-User Technology (BEUT) BBC Technology Group

From: Tom Richards @.> Reply to: guardian/grid @.> Date: Monday, 9 October 2023 at 14:19 To: guardian/grid @.> Cc: Andrew Downing @.>, Mention @.***> Subject: Re: [guardian/grid] 1443 add keywords uploadpage (PR #4139)

hi @AndyKilmoryhttps://github.com/AndyKilmory - sorry for the delay, but if you could please do one last rebase/merge of main into your branch, then CI should succeed and I can merge - thank you

— Reply to this email directly, view it on GitHubhttps://github.com/guardian/grid/pull/4139#issuecomment-1752999942, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A6UE4XXXFME3VKH6WNHCIA3X6P2U7AVCNFSM6AAAAAA4UWQIQWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONJSHE4TSOJUGI. You are receiving this because you were mentioned.Message ID: @.***>

AndyKilmory commented 11 months ago

Hi Tom, Apologies but the CI checks have failed again on #4139 – it doesn’t appear to be a coding failure but looks like the CI runner stopped or restarted for some reason?

Thanks Andy

Andy Downing (he/him) Senior Software Engineer (Images) Business Systems and Applications Broadcast & End-User Technology (BEUT) BBC Technology Group

From: Tom Richards @.> Reply to: guardian/grid @.> Date: Monday, 9 October 2023 at 14:19 To: guardian/grid @.> Cc: Andrew Downing @.>, Mention @.***> Subject: Re: [guardian/grid] 1443 add keywords uploadpage (PR #4139)

hi @AndyKilmoryhttps://github.com/AndyKilmory - sorry for the delay, but if you could please do one last rebase/merge of main into your branch, then CI should succeed and I can merge - thank you

— Reply to this email directly, view it on GitHubhttps://github.com/guardian/grid/pull/4139#issuecomment-1752999942, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A6UE4XXXFME3VKH6WNHCIA3X6P2U7AVCNFSM6AAAAAA4UWQIQWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONJSHE4TSOJUGI. You are receiving this because you were mentioned.Message ID: @.***>

twrichards commented 11 months ago

Hi Tom, Apologies but the CI checks have failed again on #4139 – it doesn’t appear to be a coding failure but looks like the CI runner stopped or restarted for some reason? Thanks Andy

no need to apologise @AndyKilmory - this has happened a couple of times, I think when GitHub Actions is busy, it kills off jobs using lots of resources - something I'll look into - for now though, I've restarted the CI job, so hopefully we can merge in 15mins or less....

prout-bot commented 11 months ago

Overdue on auth, usage, image-loader, metadata-editor, thrall, leases, cropper, collections, media-api, kahuna (created by @AndyKilmory and merged by @twrichards 30 minutes and 6 seconds ago) What's gone wrong?

prout-bot commented 11 months ago

Seen on image-loader, media-api (created by @AndyKilmory and merged by @twrichards 43 hours, 19 minutes and 40 seconds ago) Please check your changes!

prout-bot commented 11 months ago

Seen on metadata-editor, collections, cropper, usage (created by @AndyKilmory and merged by @twrichards 43 hours, 19 minutes and 46 seconds ago) Please check your changes!

prout-bot commented 11 months ago

Seen on leases (created by @AndyKilmory and merged by @twrichards 43 hours, 19 minutes and 51 seconds ago) Please check your changes!

prout-bot commented 11 months ago

Seen on thrall (created by @AndyKilmory and merged by @twrichards 43 hours, 19 minutes and 58 seconds ago) Please check your changes!

prout-bot commented 11 months ago

Seen on auth, kahuna (created by @AndyKilmory and merged by @twrichards 43 hours, 20 minutes and 3 seconds ago) Please check your changes!

paperboyo commented 10 months ago

Hi, @AndyKilmory.

Messaging here in addition to Slack (see there for details). We’ve noticed a regression bug and we verified it worked correctly before this PR:

Batch-applying Labels, People and Keywords (arrays) using a plus icon (for when not all, but only some of the selected images carry the value) stopped working (console prints error in list-editor.js). Adding a fresh label/person to a selection (where none of the selected images have it) works OK as does deleting a label/person where it’s only carried by some of the selected images.So it’s only adding that’s broken.

Untitled

AndyKilmory commented 10 months ago

Hi Mateusz, Apologies – will pick this up asap

Regards, Andy

Andy Downing (he/him) Senior Software Engineer (Images) Business Systems and Applications Broadcast & End-User Technology (BEUT) BBC Technology Group

From: Mateusz @.> Reply to: guardian/grid @.> Date: Friday, 3 November 2023 at 11:11 To: guardian/grid @.> Cc: Andrew Downing @.>, Mention @.***> Subject: Re: [guardian/grid] 1443 add keywords uploadpage (PR #4139)

Hi, @AndyKilmoryhttps://github.com/AndyKilmory.

Messaging here in addition to Slack (see there for details). We’ve noticed a regression bug and we verified it worked correctly before this PR:

Batch-applying Labels, People and Keywords (arrays) using a plus icon (for when not all, but only some of the selected images carry the value) stopped working (console prints error in list-editor.js). Adding a fresh label/person to a selection (where none of the selected images have it) works OK as does deleting a label/person where it’s only carried by some of the selected images.So it’s only adding that’s broken.

[Untitled]https://user-images.githubusercontent.com/6032869/280259017-ceadd4d5-81c4-45fb-bcb3-27a3ca40469c.gif

— Reply to this email directly, view it on GitHubhttps://github.com/guardian/grid/pull/4139#issuecomment-1792254539, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A6UE4XQMX6J4NVA3NGDJPKLYCTGOPAVCNFSM6AAAAAA4UWQIQWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOJSGI2TINJTHE. You are receiving this because you were mentioned.Message ID: @.***>

AndyKilmory commented 10 months ago

Hi Mateusz, PR in to fix this issue - https://github.com/guardian/grid/pull/4181 - apologies for the inconvenience.

Regards, Andy

Andy Downing (he/him) Senior Software Engineer (Images) Business Systems and Applications Broadcast & End-User Technology (BEUT) BBC Technology Group

From: Mateusz @.> Reply to: guardian/grid @.> Date: Friday, 3 November 2023 at 11:11 To: guardian/grid @.> Cc: Andrew Downing @.>, Mention @.***> Subject: Re: [guardian/grid] 1443 add keywords uploadpage (PR #4139)

Hi, @AndyKilmoryhttps://github.com/AndyKilmory.

Messaging here in addition to Slack (see there for details). We’ve noticed a regression bug and we verified it worked correctly before this PR:

Batch-applying Labels, People and Keywords (arrays) using a plus icon (for when not all, but only some of the selected images carry the value) stopped working (console prints error in list-editor.js). Adding a fresh label/person to a selection (where none of the selected images have it) works OK as does deleting a label/person where it’s only carried by some of the selected images.So it’s only adding that’s broken.

[Untitled]https://user-images.githubusercontent.com/6032869/280259017-ceadd4d5-81c4-45fb-bcb3-27a3ca40469c.gif

— Reply to this email directly, view it on GitHubhttps://github.com/guardian/grid/pull/4139#issuecomment-1792254539, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A6UE4XQMX6J4NVA3NGDJPKLYCTGOPAVCNFSM6AAAAAA4UWQIQWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOJSGI2TINJTHE. You are receiving this because you were mentioned.Message ID: @.***>


http://www.bbc.co.uk This e-mail (and any attachments) is confidential and may contain personal views which are not the views of the BBC unless specifically stated. If you have received it in error, please delete it from your system. Do not use, copy or disclose the information in any way nor act in reliance on it and notify the sender immediately. Please note that the BBC monitors e-mails sent or received. Further communication will signify your consent to this.


paperboyo commented 10 months ago

No need to apologise and mega-thanks for speedy fix!