Adding certain events to support concurrent editing

[ViditChitkara]

We might be needing the following events to be broadcasted on the action cable(mapknitter)
channel whenever a change is made:-

1. updateImage - Whenever an image is updated. We need to make sure the image is locked to other users when one user is making edits to it (still need to figure out how to do this).
2. deleteImage - Whenever an image is deleted.
3. addImage - On addition of an image.

The image format we would be sending with each of the above event could be of json format:-

{
    updatedImages: [
           {
                 "imageId": 12345,
                 "corners": {...}
           }, 
          {...}
    ],
    deletedImagesImages: [
           {
                 "imageId": 12345,
           }, 
          {...}
    ],
    addedImages: [
           {
                 "imageId": 12345,
                 "corners": {...}
           }, 
          {...}
    ],
}

Maybe something like this. I'm not sure though. Any ideas would be really appreciated.
More details here :- publiclab/mapknitter#609

[welcome]

Thanks for opening your first issue here! Please follow the issue template to help us help you 👍🎉😄
If you have screenshots to share demonstrating the issue, that's really helpful! 📸 You can make a gif too!

[ViditChitkara]

@jywarren @divyabaid16 @jainaman224 any ideas here?

[sashadev-sky]

@ViditChitkara don't fully understand what is needed here but if you would want to schedule a time for a phone call to walk me through I can help you implement this from the LDI end!

[sashadev-sky]

@ViditChitkara actually maybe I do understand. You just want 3 custom events fired from the LDI side, that send through the imageId and corners as json data, and can be listened for on the MK?

[ViditChitkara]

Yes Sasha, that's exactly what is wanted.
However we need to design those events carefully. I think adding and deleting an image won't cause much of a conflict.

But, what will happen if two users are making changes to the same image. Will it be that frequent?

A possible case I can think of, to prevent such conflicts is, a single user who is editing the image will have an option to lock the image editing (a button or something). Through that we'll broadcast an event to all other editors making the image locked(disabled to edit) on all other editing systems or simply add a field isLocked to json object of that particular image.

Does this make sense??
I think this still needs some thinking. If there could be any possible loophole in this? Thanks for going through this.

[sashadev-sky]

@ViditChitkara isn't the ultimate goal in MK to make people's maps associated with their accounts, so only one user is allowed to make updates to his own image anyway? Need some background on the action cable project and this group editing concept!

Currently every image does have a lock / unlock option in the toolbar.

• a custom event firing on lock / unlock is definitely a reasonable way to manage what you're saying! It could make a db update on the MK side.
• Alternatively, I think maybe the LDI interface for lock and unlock could be used just for convenience during editing. Say for making sure you don't accidentally trigger a group action on a certain image you finished editing, or just as a way of noting to yourself that this image is done with edits.
• For lock and unlock in the sense of user permissions, we could instead be locking maps from the MK side. So logged in and anonymous users can make a map and lock it. Calling img.editing.disable() on each image in the map will prevent editing, removing any handles (even lock), and any way to access a toolbar. Unlocking map can then fire img.editing.enable(). This could be a nice way to indicate maps are complete without having lock handles everywhere.

Here is a gif of the difference:

Let me know your thoughts! @jywarren would love your input here too!

[ViditChitkara]

@sashadev-sky Well, yes I think it'll make more sense if the user who uploads the image is able to edit it and not others. However the users may add multiple images and the changes could be reflected in real-time. I think that way the update of a single image issue would be automatically resolved. What do you think?
Thanks for digging into this!!!

[jywarren]

We often have folks doing a group workshop where multiple people want to edit. So, perhaps what we should do is open an issue at MapKnitter that provides a setting "allow anyone to edit this map" when you create the map, and we can make that an editable setting? We could eventually allow various permissions levels as in Google Docs.

…

On Tue, Jul 9, 2019 at 11:55 AM Vidit ***@***.***> wrote: @sashadev-sky <https://github.com/sashadev-sky> Well, yes I think it'll make more sense if the user who uploads the image is able to edit it and not others. However the users may add multiple images and the changes could be reflected in real-time. I think that way the update of a single image issue would be automatically resolved. What do you think? Thanks for digging into this!!! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#328?email_source=notifications&email_token=AAAF6JZB3B4GGOIYPA5BSGDP6SYE5A5CNFSM4H6UCVB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZQWYTY#issuecomment-509701199>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAF6J5NDCBQ4MDL62Q2SGLP6SYE5ANCNFSM4H6UCVBQ> .

[ViditChitkara]

Okay, so do we need to do the locking thing? Like when the user locks a particular selected image, no other user at that very instance shall be able to edit that image. (Adding, deleting and updating other images is fine I think). Does this make sense? Sasha demonstrated the locking/unlocking event in above comments.
Also I'll make the new issue for the editing option while creating the map.
Thanks.

[jywarren]

Ah, yes! I think we should do locking. Could we do a generic `update` event that can pass various types of image state, like transparency, locked-ness, new corner positions, etc?

…

On Tue, Jul 9, 2019 at 12:14 PM Vidit ***@***.***> wrote: Okay, so do we need to do the locking thing? Like when the user locks a particular selected image, no other user at that very instance shall be able to edit that image. (Adding, deleting and updating other images is fine I think). Does this make sense? Sasha demonstrated the locking/unlocking event in above comments. Also I'll make the new issue for the editing option while creating the map. Thanks. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#328?email_source=notifications&email_token=AAAF6JYSCI6UWEWDRFZ25FTP6S2MLA5CNFSM4H6UCVB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZQYO2Q#issuecomment-509708138>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAF6JYE5VISRYOGPNCDTXLP6S2MLANCNFSM4H6UCVBQ> .

[ViditChitkara]

Sure we can do that!! Will do that.

[jywarren]

Hi! I thought about this a bit more and think we need to loop in @divyabaid16 as well on this -- a couple complicating factors --

first, what do you think of relating this to the "image versioning" project? publiclab/mapknitter#682

Because if multiple people edit, each action could become a "version", you know?

Second, what about offline use... this is less critical but some people may want to make edits to a map even if their internet connection isn't active. Then when they get a connection, perhaps their edits would upload and generate a set of versions. I'm not sure how this would work, or if it's too complex, but wanted to put it out there for consideration.

All of the above, from multi-user editing to edit history (versions) to saved offline edits, have to do with the idea of "transactions" i guess. That is, if we have a generic transaction (i dunno if this is the right word) which is a single edit interaction by a user, we could use that concept to bring all three of these scenarios to life.

I'd love to hear from you all! I don't think this idea of 'transactions' or the 3 different use cases of it need block anyone. But if we are sure to plan around the three cases, we may save time as the project progresses. Thanks!

[ViditChitkara]

Well, I think that's a really cool idea. https://github.com/publiclab/mapknitter/blob/8a4d204875a1b07024cf1822a0a1559475400050/app/controllers/images_controller.rb#L73-L97 -> This is called whenever an image is placed somewhere on the map.
Along with this, four nodes are created which tells the exact position of that image. What if we call some method(in the update method itself) to fetch:-
Map -> All the warpables associated with the map -> The final nodes of each warpable.
And broadcast all this information to each consumer(users who are editing the map) using the action cable channel we created here (yet to be merged). We also need to think, how the maps would be updated on each user's system (probably some kind of update event to update the map on the client side).
This way we won't need to handle the three events (add, delete or update separately).
And for the revision history. Do you think we should have a separate table which would store the warpable_id, nodes string and a version number? I think that would be quite an easy way to get the historical record of the edits made to the image.

[ViditChitkara]

@jywarren @divyabaid16 @sashadev-sky what do you think?

[jywarren]

Cool -- so, would this work directly peer to peer as well? Whether it goes Browser => Rails => Browser or Browser => Browser, we should figure out a JSON 'transaction' format that will work.

…

On Wed, Jul 10, 2019 at 4:16 PM Vidit ***@***.***> wrote: @jywarren <https://github.com/jywarren> @divyabaid16 <https://github.com/divyabaid16> what do you think? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#328?email_source=notifications&email_token=AAAF6J5DGLIY5L26MUFJQKTP6Y7OXA5CNFSM4H6UCVB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZUTOSA#issuecomment-510211912>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAF6J3J2NYZO6FU2BVGGG3P6Y7OXANCNFSM4H6UCVBQ> .

[ViditChitkara]

I think the first one:-
Browser to rails (The normal update method is called) -> Fetch the images and get the required transaction format (rails) -> Rails method broadcasts it to users.
And yes, we need some ideas on JSON 'transaction' format and more importantly how we are going to use that json to update map on each user's site. Like should we reload the page or manually put the images as the json directs us. In the meanwhile should I start writing the method which will generate the json and will be called inside image_controller#update method? I think this has to be done in the action cable pull request. Does it make any sense?
This is really interesting, I think are we are one step closer now!!

[jywarren]

Let's hear from DivyaBaid16 first, but this is sounding good! Wouldn't the JSON transaction be generated in JS on either end, even if it's relayed through Action Cable, rather than building JSON in rails?

…

On Wed, Jul 10, 2019 at 5:40 PM Vidit ***@***.***> wrote: I think the first one:- Browser to rails (The normal update method is called) -> Fetch the images and get the required transaction format (rails) -> Rails method broadcasts it to users. And yes, we need some ideas on JSON 'transaction' format and more importantly how we are going to use that json to update map on each user's site. Like should we reload the page or manually put the images as the json directs us. In the meanwhile should I start writing the method which will generate the json and will be called inside image_controller#update method? I think this has to be done in the action cable pull request <publiclab/mapknitter#805>. Does it make any sense? This is really interesting, I think are we are one step closer now!! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#328?email_source=notifications&email_token=AAAF6J5K6QQUONMTNUCRSCLP6ZJL7A5CNFSM4H6UCVB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZU2FWQ#issuecomment-510239450>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAF6JZUWOLVXHWQN5VSEQTP6ZJL7ANCNFSM4H6UCVBQ> .

[ViditChitkara]

I think we can do it either ways. Doing it on the rails server would just be a method call '.to_json' or something. So it'll be like:-

update method called -> rails collects the warpables for the map and create a json, finally broadcasting it to all other connected browsers -> use the captured json by browsers to make realtime changes.(This will likely be done by JS on all browsers, will need more details here)

Am I thinking it correctly or should there be some other flow?
Thanks and yes let's wait for divya before starting coding on this!!

[jywarren]

Cool! My preference is slightly for a single means, so that we use the same code for peer-to-peer connections like using https://peerjs.com/ as we do in the Rails version where it'd be relayed by Rails. It just seems a little simpler than intercepting and re-building in Rails if we're able to generate them on the client side anyways, and we do have to watch for performance hits if tons of these are being relayed back and forth! I did want to ask if you'd taken a look at https://peerjs.com/ because I could see how it'd have a performance impact to avoid all these passing through the Rails server...?

…

On Wed, Jul 10, 2019 at 5:57 PM Vidit ***@***.***> wrote: I think we can do it either ways. I think doing it on the rails server would just be a method call '.to_json' or something. So it'll be like:- update method called -> rails collects the warpables for the map and create a json, finally broadcasting it to all other connected browsers -> use the captured json by browsers to make realtime changes.(This will likely be done by JS on all browsers, will need more details here) Am I thinking it correctly or should there be some other flow? Thanks and yes let's wait for divya before starting coding on this!! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#328?email_source=notifications&email_token=AAAF6J46QHWLVT6CP2NVRF3P6ZLJHA5CNFSM4H6UCVB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZU3PPA#issuecomment-510244796>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAF6J6OC6JDR4UC4GG46KLP6ZLJHANCNFSM4H6UCVBQ> .

[ViditChitkara]

Yes, I had a look at peerjs.com . But it occurs to me, wouldn't it be really complicated when we consolidate the edits of each user who is connected and send it to other peers? Maybe the peerjs server(which just acts as a connection broker) handle it but I think the edits we compile manually would be less reliable as to what we fetch from the database itself.

[ViditChitkara]

In addition to that, with action cable we could handle authentication and the app logic with a lot of ease as compared to an external service like peerjs. What do you think??

[divyabaid16]

Hi everyone. I agree with @ViditChitkara what he is trying to say.
And one more thing that bothered me was like if a user would not like anyone to edit the map, he must be given that privilege. What I mean is that when there is a group project only the people working in that group must be allowed to update/edit the images. Because currently, anyone can do so.
What if we add the option of who all can edit the images so that the one who has originally edit the images don't need to worry if someone else had edited. Because sometimes spammers around.

And by default, anyone can edit the images but there can be a setting like "Only I can edit the images" or "Only this, this person can edit the images".

What are your views on this?

[jywarren]

Hi, ok this is such a good point:

Maybe the peerjs server(which just acts as a connection broker) handle it but I think the edits we compile manually would be less reliable as to what we fetch from the database itself.

Like, Rails is the 'decider' so to speak of what is the official version. Peer.js can't do this. That makes sense.

@divyabaid16 also great point. We can do a lot on this by making "editable by others" a per-map setting as in publiclab/mapknitter#84, a long-desired feature. It'd be great if you wanted to take this up, it shouldn't be too hard in the basic version. And then we can worry a bit less about that part. (although a system with a token url as described there might be a little more involved).

Thanks, all!

[jywarren]

So, perhaps we really do need to build the transaction handling in MapKnitter's codebase and this repo would contain only the events. Still, we may need to think about an "undo" button on each image that lets it go back to a previous version. That'd be integration to plan ahead for.

[ViditChitkara]

Great! I'll build a method which broadcasts the transaction Json after the update method in image controller is called.
The next part will be that, we'll have to figure out how to use this json and rebuild the map on the browser itself. I'll make a separate issue on it.
Thanks, will start with step 1 now.

[ViditChitkara]

Will be doing it on the action cable branch only as it requires broadcasting.