Order when creating new ideas doesn't seem quite deterministic

[PragTob]

While playing around with the application I noticed that the insert point for new cards changed as I moved cards around. I also believe I once witnessed a lot of cards changing order instantly but I couldn't reproduce that so far.

Current Behaviour

Steps to reproduce:

1. Create a couple of cards, new cards are inserted at the beginning (top left)
2. now move a card around (I moved the middle to be foremost)
3. now the next new card is inserted at the end (right bottom)
4. further cards are now inserted before the last card

see the gif (which was recorded surprisingly easily using peek

Expected Behaviour

I'd expect that new cards would always be inserted at the same spot, whether that's at the beginning, or more likely (imo) at the end (so that no other cards need to move because of it)

[JannikStreek]

@PragTob for me this works now, maybe you can check on master if the issue is still present for you? My new ideas are always appended to the end.

EDIT: AH now it also happened to me! I think it's due to "null" as initial position_order. We would need to increment this by + 1 OR add a second sorting property (created_at) which also should work. Actually, I already implemented some logic to increment the position order, but only for the lanes and not for ideas. I think just adding created_at as the second sorting argument might be easier.

This is the part where it happens (ideas.ex, list_ideas_for_brainstorming):

    idea_query =
      from idea in Idea,
        left_join: idea_count in subquery(idea_count_query),
        on: idea_count.idea_id == idea.id,
        where: idea.brainstorming_id == ^id,
        order_by: [
          asc_nulls_last: idea.position_order,
          desc: idea.updated_at
        ]

Means: Ideas with a position order (1,2,3) are top, then the null positions follow with the newest one (updated_at) at the top, so (1,2,3, NOW, NOW-1.hour) etc. Thats how the strange ordering is happening.

[PragTob]

@JannikStreek instead of secondary order properties shouldn't we try to make position_order always have a meaningful value? I think everything else is just a band aid. Creation may work for small examples, but once you move around old cards you also run into a weird state again (I'm pretty sure), updated_at also doesn't work as that also breaks with text updates etc.

I think filling position_order correctly is the way once we drag&drop and insert it. That said, that'd mean an update to the position order of all other cards which sounds annoying but :shrug: I wonder if there is some sort of good solution. I mean we can start with 0 1000 2000 3000 and then give it a position of like 1500 that should... work for a while. There's probably smarter ideas and people out there though.

[JannikStreek]

I agree, adding a correct value by default to the position_order would be good. Especially as I already did the same for lanes (max current value +1). However, I think a secondary order is never wrong. For instance: When we migrate existing servers, this field is new and therefore no order at all is given. Either you also need to adjust the migration now and create an order for every idea for all existing ideas or just accept that there will be null values present. I am fine with the latter, but as I said, a calculated default for new ideas is good.

Creation may work for small examples, but once you move around old cards you also run into a weird state again (I'm pretty sure), updated_at also doesn't work as that also breaks with text updates etc.

Do you have examples for this, I am not sure what you mean? Newly added Ideas should be added to the end. Updated_at is not used for ordering with null values present. We are first ordering by position_order and then we look at the insertion date.

I think filling position_order correctly is the way once we drag&drop and insert it. That said, that'd mean an update to the position order of all other cards which sounds annoying

We already do that, or what do you mean? When you move a card, all positions inside a lane are freshly calculated. Null values currently only occur, when you add completely new ideas without dragging and / or ordering.

[PragTob]

@JannikStreek I was unaware that we always fill it, because if we always fill it - then why would we need a second value? And imo we should also add a position order when we just add ideas to the end of the list.

I need to review the positioning code, if we always fill the code and update the position of all other fields then it's fine. But if we don't update other position orders (I don't know right now, sorry) then the order by creation data can get wrong once you drag it around.

I'd strongly advocate for always filling position_order though so that we don't have to rely on inserted_at though. Either by running a data migration to set position_order based on inserted_at or saying "sorry this just gets a high number so it's on the end now as it was missing that"