Edit records

[n1k0]

WiP, lots of things should be done a better way. The Form component approach isn't easy to deal with; like, at all.

Deployed to gh-pages.

[magopian]

Woah, this new "submit review" feature of github needs some more work it seems: no way to preview it before submitting it, and no way to edit it afterwards...

Anyway, here are the questions: 1/ are you GETing the record from the server before editing it to make sure it's not been edited by someone else? Otherwise it would make sense to just populate the form with the content of the record we already have locally in the records list 2/ we might want to switch the data type for records to be a dict, with the key being the id.

I played around implementing the edit functionality yesterday, and for that added a new key to the model: editingRecord : Maybe RecordId. Clicking on the edit button would then populate this with the id of the record being edited, populate the form with the title/desc of the record, and then submitting the form would either create or update depending on the presence of an id in there.

What do you think?

The only thing I see missing is a way to "cancel" the editing of a record, to be able to create one instead. Also, I quite liked emptying the form on submit, so it's a clean slate for the next action (creating or updating), but I see you removed that, was there a reason?

[n1k0]

Thanks for the review!

1/ are you GETing the record from the server before editing it to make sure it's not been edited by someone else?

Yes. In my experience with the admin, this is a good way to minimize conflicts.

2/ we might want to switch the data type for records to be a dict, with the key being the id.

Could you expand on what would be the benefits?

I played around implementing the edit functionality yesterday, and for that added a new key to the model: editingRecord : Maybe RecordId. What do you think?

Yeah I debated that, went with moving editing state to the Form model itself, but not quite sure about it. My main fear is still to bloat the root model. I'll think about it more.

The only thing I see missing is a way to "cancel" the editing of a record, to be able to create one instead.

Yeah, we need to add a "New record" link or something somewhere.

Also, I quite liked emptying the form on submit, so it's a clean slate for the next action (creating or updating), but I see you removed that, was there a reason?

Because when you edit a record and submit it, you usually still want the data to stay in the form. Well, I'm not sure anymore, I may just reintroduce it :)

[magopian]

Could you expand on what would be the benefits?

It might make it easier to update the records (or get them to be put in the Form for edition, but I saw your answer regarding the GETing first, so this might not be as relevant). Instead of List.maping on all the records, we would instead do something like:

> d = Dict.fromList [("1", "foo"), ("2", "bar")]
Dict.fromList [("1","foo"),("2","bar")] : Dict.Dict String String
> Dict.update "1" (\_ -> Just "crux") d
Dict.fromList [("1","crux"),("2","bar")] : Dict.Dict String String

[n1k0]

It might make it easier to update the records

So I've been playing with using a Dict to store the records list, and TLDR: nightmare.

Sure it's easier to remove an entry:

removeRecordFromList : Record -> Records -> Records
removeRecordFromList { id } records =
    Dict.remove id records

Beautiful.

Now, updating an entry? Well, update fn signature being (Maybe a -> Maybe a) (for some very obscure reason relating to deleting through updates it seems), you need this kind of crap:

updateRecordInList : Form.Model -> Records -> Records
updateRecordInList formData records =
    -- This enables live reflecting ongoing form updates in the records list
    case formData.id of
        Nothing ->
            records

        Just id ->
            Dict.update id (updateRecord formData) records

updateRecord : Form.Model -> Maybe Record -> Maybe Record
updateRecord formData record =
    case record of
        Nothing ->
            record

        Just record ->
            Just
                { record
                    | title = Just formData.title
                    , description = Just formData.description
                }

I hope I've just missed something obvious and there's a much cleaner way to achieve this, but still.

Last, a Dict.toList always order by keys ASC (so long, last_modified), so I can't see how it's much more practical than using a List which makes dealing with sorting much easier.

I have a broken, ugly patch I can share but I'd really like to see your take on the implementation, if you have any at hand to share.

[magopian]

I believe it doesn't make sense to switch to Dict then, at least not until we have a good reason to do so. It just add (a "lot") more burden for updating, then sorting before displaying, just to be able to remove an entry more easily.

It would make sense to switch to a dict for performance reasons (O(1) lookup instead of O(n) because of the mapping on the whole list), but I doubt we have any performance issues for the moment ;)

[magopian]

Just for reference, here's the way we could sort the records by "updated":

> d = Dict.fromList [(1, {title="foo", desc="bar", updated=123}), (2, {title="crux", desc="baaz", updated=4})]
Dict.fromList [(1,{ title = "foo", desc = "bar", updated = 123 }),(2,{ title = "crux", desc = "baaz", updated = 4 })]
    : Dict.Dict
        number { desc : String, title : String, updated : number' }

> Dict.values d |> List.sortBy .updated
[{ title = "crux", desc = "baaz", updated = 4 },{ title = "foo", desc = "bar", updated = 123 }]
    : List { desc : String, title : String, updated : number' }

or if we want to keep the tuples with the IDs (for some reason) using function composition:

> Dict.toList d |> List.sortBy (snd >> .updated)
[(2,{ title = "crux", desc = "baaz", updated = 4 }),(1,{ title = "foo", desc = "bar", updated = 123 })]
    : List
        ( number, { desc : String, title : String, updated : number' } )

[n1k0]

After experimenting with the approach described in https://github.com/n1k0/kinto-elm-experiments/pull/8#issuecomment-248957914, we decided to go with using a Dict in https://github.com/n1k0/kinto-elm-experiments/pull/8/commits/f62415b377afd203a67327aa9e44500b0585aa45.