appsmith
appsmith copied to clipboard
[Feature]-[4000]:Autosave for new datasources needs improvements
Description
In Appsmith we try to autosave everywhere. Today when a user clicks on a new datasource, it creates new named resource even though the user might have just been exploring.
Solution
We should not save the datasource if there was nothing typed inside the form. It should just disappear because the user was exploring and not actually creating a new datasource.
Can i work on this issue?
@mandvisingh go for it! Do read our contribution guidelines and raise a PR within 4 days :)
Can you point me to where the code for the autosaver is at? I tried searching through the code but couldn't quickly find it. If it's as simple as wrapping it in an if value!='' statement, I am happy to take this one and can do it within 4 days!
@brittanyjoiner15 Unfortunately, this issue involves a lot of refactoring and doesn't meet our criteria of what qualifies as a good first issue. This issue will be picked up by one of our internal team members.
@arunvjn ah, i see! good luck!
datasources created, 6m: 12000 assume 1/3 of users go away from the configuration page with unsaved changes: 4000
Stats
| Stat | Values |
|---|---|
| Reach | 4000 |
| Effort (months) | < |
datasources created, 6m: 12000 assume 1/3 of users go away from the configuration page with unsaved changes: 4000
Stats
| Stat | Values |
|---|---|
| Reach | 4000 |
| Effort (months) | 0.5 |
design link: https://www.notion.so/appsmith/Autosave-vs-Manual-save-in-Datasource-fb96a01c3df74a41a4bd0c63398188fe
User flows
Create Mode Intends to create, intends to save
- I click on new datasource
+and select the plugin to see the create datasource form - I should be able to enter the fields I want, then click save, so that the datasource is created
Intends to create, does not intend to save In this case, I should be prompted to save
- I click on new datasource
+and select the plugin to see the create datasource form - I click the
BackorCancelbutton with or without making any changes to the form - I should be prompted to ensure whether I want to save my changes or not, so that: a. yes: the datasource is saved b. no: the datasource is not saved, I am taken to the previous page from where I navigated here also discussed here and here
Does not intend to create
- I click on new datasource
+and select the plugin to see the create datasource form - . I should be able to use the
BackorCancelaction, so that by clicking this, I can go back to the previous page from where I navigated here - the datasource should not get created
Edit Mode Intends to view only
- When I click on
Editfor a datasource, I should see the datasource form in edit mode - The
Savebutton should be disabled by default - I should be able to navigate back to the previous page using
BackorCancel
Intends to make changes and save
- When I click on
Editfor a datasource, I should see the datasource form in edit mode - The
Savebutton should be disabled by default - I should be able to make changes, upon which the
Savebutton should get enabled - When I click save, my latest changes should get saved to the datasource
Intends to make changes and not save
- When I click on
Editfor a datasource, I should see the datasource form in edit mode - The
Savebutton should be disabled by default - Now if I hit
BackorCancel, I should be prompted to ensure whether I want to save my changes or not, so that: a. yes: the datasource is saved b. no: the datasource is not saved
@rohan-arthur I have a couple of questions regarding this functionality:
- so currently suppose if I create a new datasource and instead of saving that datasource, user reloads the page, we get the same datasource page in view mode, which user can then edit, but if we are planning to create datasource only when user clicks on save button, then what would happen if user reloads the page in such case before actually saving the datasource?
- We have delete and test buttons along with save button in datasource configuration form, but if we are planning to create datasource only on the click of save button, then what would happen if user clicks on delete/test button before clicking on save and creating the datasource? In this case should we hide/disable these buttons?
CC: @AmanAgarwal041
@sneha122 :
- close tab, refresh should be handled similarly but let's keep it out of scope for this issue - since its incidence is lesser and it is complicated to implement. For now, the changes should simply not be saved if I close or refresh the tab. If on refresh, the default behaviour is retaining the user's changes, but they are not saved yet - even so, let's go with it.
- Test should be possible without save, so there need not be any change.
- Delete, you're right - this does not make sense. Can we disable the delete if no save has happened yet in create mode?
@rohan-arthur @sneha122
- When creating a datasource, we should not switch to view mode after clicking the save button; instead, we should stay in that mode and enable the test button. By doing this, the user can test the datasource with just one click.
- When a user creates a datasource, we are aware of the path. In order to keep the last altered state when a user refreshes the page, we may either build a new temporary datasource on the client or save the updated data in local storage in the same way that it was previously saved on the server.
@AmanAgarwal041 that doesn't sound like a good UX because the feedback on saving the datasource is not very strong. When a user saves the datasource, we can automatically test it and assign it a state as
- Successfully connected
- Error in connecting
- Not testable
@AmanAgarwal041 Moreover currently when we save the datasource it takes us back to active datasource list, and not in the datasource config viewMode, do we need to alter that behaviour as well?
@AmanAgarwal041 Moreover currently when we save the datasource it takes us back to active datasource list, and not in the datasource config viewMode, do we need to alter that behaviour as well? @rohan-arthur Any thoughts here ?
@sneha122 @AmanAgarwal041 after saving, ideally I should see the ds that I just saved. If this is not happening, please create a separate issue for this (looks like this issue has large enough scope already)
@sneha122 @rohan-arthur @AmanAgarwal041
-
As per my understanding the data in the form is saved in local storage and it persists till the point when the session is reloaded or killed (when I refresh the tab or kill the tab). Aside from these two options even if i move from the data source page to canvas or anywhere else and I come back to edit the data source the value still persists. Since the value persists it conveys the message that it's saved and there is no indicator if it's in local storage or saved on the server. Reference Video.
Keeping this in mind, are we still going to continue saving the value in local storage if so then we need some indicator to covey this or are we going to destroy the value if the user exists the configuration form without saving? IMO we should not have values persists locally when the user has moved away from the data source configuration form, this way we explicitly tell the users to save the values before exiting. -
Agreed with @rohan-arthur that the delete option should be disabled when the data source is not saved.
-
@rohan-arthur after saving the data source it should lead back to the review page of the datasource created where we provide the option to create a new query/API. We should pick this up with this since it's part of the core experience.
As per the discussion, we would go with following approach for datasource autosave:
- Delete button will be disabled until user saves the datasource for the first time
- Test button will remain enabled, as user should be able to test the datasource without actually having to save the datasource
- Whenever user moves from create datasource/edit datasource configuration page without saving the data, then they would be shown warning popup as to save the data or dont save the data. If dont save is clicked, all temporary datasource data/edits made by user to existing datasource will be lost.
@AmanAgarwal041 that doesn't sound like a good UX because the feedback on saving the datasource is not very strong. When a user saves the datasource, we can automatically test it and assign it a state as
- Successfully connected
- Error in connecting
- Not testable
@Nikhil-Nandagopal I would be creating a separate ticket for this CC: @rohan-arthur @AmanAgarwal041 @parth-appsmith
@rohan-arthur Here are a couple of the points not taken into account/increased the scope of this ticket:
- Save as datasource for REST and GraphQL API flow
- After save earlier it used to go active DS list, but now it needs to stay in DS config page with view mode, Although we discussed that we can take this change later on. since in the discard popup designs also, on save it was going on view mode ds config page, I made that change
- When we import applications, we have datasource reconnect popup, where we need to configure datasources again.
While checking the failed cypress tests for datasource autosave improvements, I came across one test in globalsearch_spec.js, which attempts to navigate to same datasource while its creation, following is the video attached: With new implementation, since we save datasource only on save button click, what should we do in such case? cc: @rohan-arthur @sumitsum @AmanAgarwal041 @parth-appsmith
https://user-images.githubusercontent.com/30018882/202695918-ba64e1b8-1378-43e7-a99c-f537947afcb9.mov
While checking the failed cypress tests for datasource autosave improvements, I came across one test in globalsearch_spec.js, which attempts to navigate to same datasource while its creation, following is the video attached: With new implementation, since we save datasource only on save button click, what should we do in such case? cc: @rohan-arthur @sumitsum @AmanAgarwal041 @parth-appsmith
Screen.Recording.2022-11-18.at.5.01.55.PM.mov
I believe that the global menu should not list the same datasource because the new datasource has not yet been created without pressing the save button. Can we anticipate this behaviour from the test suite?
Yes we can fix this in a way that datasource itself wont be shown in the list, because it has not been created yet. But I think there would be issues in replicating this behaviour in test suite because we do not have tests for google sheets authentication yet.
@sneha122 sorry I couldn't understand this part - But I think there would be issues in replicating this behaviour in test suite because we do not have tests for google sheets authentication yet. Can you please explain this ?
@AmanAgarwal041 sorry I couldn't understand this, can you please explain once ? Can we anticipate this behaviour from the test suite?