That’s not my name #3048
That’s not my name #3048
Conversation
| [*.html] | ||
| indent_size = 4 |
There was a problem hiding this comment.
I added this to make my editor happy, but happy to drop this change.
There was a problem hiding this comment.
You must have a wide screen 😅 .
There was a problem hiding this comment.
I'm working full screen with 1440 pixels. That 4 spaces is in line with what's currently used in the Angular html files.
| @@ -20,15 +20,15 @@ describe('metadataDiff', () => { | |||
| it("finds a changed string field in the presence of an array", () => { | |||
| const initial = { field: "value", array1: ["value 2"] }; | |||
| const changed = { field: "changed", array2: ["value 2"] }; | |||
| const expected = { field: "changed" }; | |||
| const expected = { field: "changed", array1: "", array2: ["value 2"] }; | |||
There was a problem hiding this comment.
Shouldn't array1 be absent from expected here?
There was a problem hiding this comment.
I think we array1 should return an empty array. In any case, I’ve updated the tests.
| value.map { case(k, v) => (k, if (v == "") null else v) } | ||
| .foreach { case(k, v) => valueMap.withString(k, v) } | ||
| value.map { case(k, v) => (k, if (v == JsNull) null else v) } | ||
| .foreach { case(k, v) => valueMap.withJSON(k, Json.stringify(v)) } |
There was a problem hiding this comment.
Would be good to have some tests against this change.
There was a problem hiding this comment.
@akash1810 I think this is done now?
|
Grid CI / ScalaBuild (pull_request) failed on |
Co-Authored-By: Akash Askoolum <[email protected]>
|
👍 Happy with this, have seen it on TEST and it's writing sensible things to the database. @akash1810 are you happy with the tests? |
There was a problem hiding this comment.
This change is great! Not least because it addresses a ~5 year issue!
Works as described on TEST; perhaps @paperboyo wants to give it a final try? We should also send some comms out via Central Production.
This is also likely to be of interest to BBC so cc @ochiengolanga and @wainaina. We can also bring it up at our next meeting.
| [*.html] | ||
| indent_size = 4 |
There was a problem hiding this comment.
You must have a wide screen 😅 .
Him did already. Works a treat!
Defo! cc @marianadiaspereira
Would that be editing arrays?
Yeah, they have this cool thing (although I can’t tell which fields are possible to add and if |
prout-bot
added
Pending-on-image-loader-projection
Pending-on-kahuna
Pending-on-leases
Pending-on-media-api
Pending-on-metadata-editor
Pending-on-usage
Seen-on-auth
Seen-on-collections
Seen-on-cropper
Seen-on-image-loader-projection
Seen-on-kahuna
Seen-on-leases
Seen-on-metadata-editor
and removed
Pending-on-auth
Pending-on-collections
Pending-on-cropper
Pending-on-image-loader-projection
Pending-on-kahuna
Pending-on-leases
Pending-on-metadata-editor
labels
|
Seen on auth, cropper, collections, kahuna, image-loader-projection, metadata-editor, leases (created by @mxdvl and merged by @paperboyo 10 minutes and 44 seconds ago) Please check your changes! |
|
Seen on usage, image-loader, media-api (created by @mxdvl and merged by @paperboyo 10 minutes and 49 seconds ago) Please check your changes! |
What does this change?
Adds the ability to edit the People in an image. Developed with @zeek01 as part of Hack Day 2020.
Some of the “prompted change” alluded to in Mistakes with identities have dismayed our readers – and prompted change.
This required to change the underlying Scala type from
Map[String, String]→Map[String, JsValue]. Theservice.jsalso handle thepeopleInImagefield differently than other.How can success be measured?
Picture editors should be able to correct this metadata if it is wrong.
Screenshots
Who should look at this?
@paperboyo, @akash1810, @guardian/digital-cms
Tested?