The-Commit-Company / frappe-react-sdk

React hooks for Frappe
https://frappe-react.vercel.app
MIT License
110 stars 36 forks source link

feature:frappe-react-indexdb #12

Closed sumitjain236 closed 8 months ago

nikkothari22 commented 1 year ago

A few points that come to my mind as of now:

  1. Local storage needs to be cleared when the user is logged out. Currently the library only clears it when the user explicitly logs out using the in-built function. But a user can be logged out or account disabled by the admin from the backend. In that case, we need to check for the logged in state and clear the storage if the user is Guest.
  2. Permissions can be changed by the admin for a particular user. In that case, the system needs to clear data from local storage as well. Currently we are only checking timestamp, but we need to find a way to validate whether a user has the necessary permissions to access the documents or not, and if not, it should clear the storage. We need to think this through.

Both of the above points are critical before this can be merged. We need to ensure that we do not store data that the user does not have access to.

sumitjain236 commented 1 year ago

A few points that come to my mind as of now:

  1. Local storage needs to be cleared when the user is logged out. Currently the library only clears it when the user explicitly logs out using the in-built function. But a user can be logged out or account disabled by the admin from the backend. In that case, we need to check for the logged in state and clear the storage if the user is Guest.
  2. Permissions can be changed by the admin for a particular user. In that case, the system needs to clear data from local storage as well. Currently we are only checking timestamp, but we need to find a way to validate whether a user has the necessary permissions to access the documents or not, and if not, it should clear the storage. We need to think this through.

Both of the above points are critical before this can be merged. We need to ensure that we do not store data that the user does not have access to.

Updated Code added

socket-security[bot] commented 1 year ago

Socket Security Pull Request Report

👍 No new dependency issues detected in pull request

Pull request report summary
Issue Status
Install scripts ✅ 0 issues
Native code ✅ 0 issues
Bin script confusion ✅ 0 issues
Bin script shell injection ✅ 0 issues
Unresolved require ✅ 0 issues
Invalid package.json ✅ 0 issues
HTTP dependency ✅ 0 issues
Git dependency ✅ 0 issues
Potential typo squat ✅ 0 issues
Known Malware ✅ 0 issues
Telemetry ✅ 0 issues
Protestware/Troll package ✅ 0 issues
Bot Commands

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore foo@1.0.0 bar@2.4.2

Powered by socket.dev

sumitjain236 commented 1 year ago

A few points that come to my mind as of now:

  1. Local storage needs to be cleared when the user is logged out. Currently the library only clears it when the user explicitly logs out using the in-built function. But a user can be logged out or account disabled by the admin from the backend. In that case, we need to check for the logged in state and clear the storage if the user is Guest.
  2. Permissions can be changed by the admin for a particular user. In that case, the system needs to clear data from local storage as well. Currently we are only checking timestamp, but we need to find a way to validate whether a user has the necessary permissions to access the documents or not, and if not, it should clear the storage. We need to think this through.

Both of the above points are critical before this can be merged. We need to ensure that we do not store data that the user does not have access to.

Updated Code added

python repository : https://github.com/sumitjain236/frappe_offline.git

tsmanagers commented 10 months ago

Do we need to use fraape_offline app along with frappe-react SDK?

nikkothari22 commented 10 months ago

@tsmanagers

Caching on IndexedDB is not yet supported by the SDK. We were trying to build something similar to Replicache and Linear, but we realised that it cannot be a generic solution. (Two way real-time sync is hard)

This is something that could just be a separate library (not dependent on Frappe at all) and should ideally be implemented based on the application.