Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(organizations): code cleanup and patch data cleanup in account settings and tos view TASK-1292 #5290

Merged
merged 11 commits into from
Nov 28, 2024

Conversation

pauloamorimbr
Copy link
Contributor

@pauloamorimbr pauloamorimbr commented Nov 21, 2024

👀 Preview steps

  1. ℹ️ have an account and a project
  2. Go into account settings view
  3. Change information on some field(s)
  4. Save the changes
  5. Check in the network tab the call to the /me endpoint. The payload should only contain the changed fields.
  6. Enable a TOS view by following this document
  7. In the TOS view, fill in the information and save (you may want to preserve log before saving data)
  8. Check the network tab. Payload should contain only the changed field.

💭 Notes

We're refactoring accountDetails and TOS view on the way of implementing some changes on displayed fields:

@pauloamorimbr pauloamorimbr self-assigned this Nov 21, 2024
@pauloamorimbr pauloamorimbr changed the title refactor(organizations): refactor account settings ans tos pages TASK-1292 refactor(organizations): cleanup and cleanup patch data in account settings and tos view TASK-1292 Nov 21, 2024
@pauloamorimbr pauloamorimbr changed the title refactor(organizations): cleanup and cleanup patch data in account settings and tos view TASK-1292 refactor(organizations): code cleanup and patch data cleanup in account settings and tos view TASK-1292 Nov 21, 2024
Copy link
Contributor

@jamesrkiger jamesrkiger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left one comment, but otherwise lgtm

@pauloamorimbr pauloamorimbr marked this pull request as draft November 26, 2024 16:12
pauloamorimbr added a commit that referenced this pull request Nov 27, 2024
### 👀 Preview steps
No behavior will change with this implementation, but it can be tested
by using the new hook somewhere.
As a test I've modified `MyProjectRoute` to verify that
`currentLoggedAccount` was properly dispatching component re-render and
that using hooks methods are working properly as well:
```typescript
...
export default function MyProjectsRoute() {

  const [counter, setCounter] = React.useState(0);

  const {currentLoggedAccount, refreshAccount} = useSession();

  useEffect(() => {
    setCounter((prev) => prev + 1);
  }, [currentLoggedAccount]);

  return (
    <>
    <button onClick={refreshAccount}>Refresh {counter} - {currentLoggedAccount?.username}</button>
    <UniversalProjectsRoute
...
```
Result:

![KPI_5303](https://github.com/user-attachments/assets/be5f9c7f-ce37-459b-90a7-9848bb88db7d)

### 💭 Notes

- This PR adds the `useSession` hook, aimed to be used instead of the
current `sessionStore` class to access the session data and methods.
- The `useSession` hook returns the `currentLoggedAccount` user, which
is already filtered to be an existent account returned from the `/me`
endpoint and verified not to be an anonymous account, which is currently
done in several places around the codebase where the `currentAccount`
from `sessionStore` is used.
- The internal implementation of this hook will eventually migrate to
use `react-query`, keeping the hook's signature.
- This implementation can be used as a base for the migration of the
other existing stores to allow for future dropping of `mob-x` in favor
of using react-query to cache data.
- First usage of this hook can be seen in
#5290
@pauloamorimbr pauloamorimbr force-pushed the task-1292-refactor-account-settings-and-tos branch from d64e43a to 50de831 Compare November 27, 2024 19:49
@pauloamorimbr pauloamorimbr marked this pull request as ready for review November 28, 2024 12:36
@pauloamorimbr pauloamorimbr force-pushed the task-1292-refactor-account-settings-and-tos branch from 7c910b9 to 0036e25 Compare November 28, 2024 12:45
@pauloamorimbr pauloamorimbr merged commit aaa8aac into main Nov 28, 2024
7 checks passed
@pauloamorimbr pauloamorimbr deleted the task-1292-refactor-account-settings-and-tos branch November 28, 2024 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants