Skip to content
This repository has been archived by the owner on Jan 16, 2022. It is now read-only.

fix: package list refresh based on logged-in user #415

Merged
merged 4 commits into from
Jan 12, 2020

Conversation

coolsp
Copy link
Member

@coolsp coolsp commented Jan 9, 2020

Type: fix

Description:

In pages/home/Home.tsx now monitoring any change in a user log-in/out which will trigger a new API.request to get the packages from the Verdaccio-server. This is done by creating a useEffect on isUserLoggedIn. Code has been transplanted from App/App.tsx including the use of the Loading component during the XHR. The use of packages was removed from other components as no longer needed and tests updated.

Resolves #414

description:
In `pages/home/Home.tsx` now monitoring any change in a user log-in/out which will trigger a new `API.request` to get the _packages_ from the Verdaccio-server.  This is done by creating a `useEffect` on **isUserLoggedIn**.  Code has been transplanted from `App/App.tsx` including the use of the Loading component during the XHR.  The use of **packages** was removed from other components as no longer needed and tests updated.

Resolves issue #414
@juanpicado juanpicado self-requested a review January 10, 2020 05:42
juanpicado
juanpicado previously approved these changes Jan 12, 2020
Copy link
Member

@juanpicado juanpicado left a comment

Choose a reason for hiding this comment

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

Thanks @coolsp good job

There is something we need to keep on mind and worth to document on the website. In our local dev config we have the following

security:
  api:
    jwt:
      sign:
        expiresIn: 120d
        notBefore: 1
  web:
    sign:
      expiresIn: 100d
      notBefore: 1

Let's focus on notBefore: 1 for web section for a second. According the docs
https://github.com/auth0/node-jsonwebtoken#jwtsignpayload-secretorprivatekey-options-callback

notBefore: expressed in seconds or a string describing a time span zeit/ms.
Eg: 60, "2 days", "10h", "7d". A numeric value is interpreted as a seconds count. If you use a string be sure you provide the time units (days, hours, etc), otherwise milliseconds unit is used by default ("120" is equal to "120ms").

If the user has something different than notBefore: 0 it will get the following error in the console and the refresh won't work (which is the case of this repo https://github.com/verdaccio/ui/blob/master/tools/_verdaccio.config.yaml#L26).

Screen Shot 2020-01-12 at 6 23 59 PM

So, as I suggested, worth to document it either

@juanpicado juanpicado requested review from a team January 12, 2020 17:38
juanpicado added a commit that referenced this pull request Jan 12, 2020
@juanpicado
Copy link
Member

juanpicado commented Jan 12, 2020

Please run yarn test:update-snapshot and push again, snapshots are outdated.

description:
In `pages/home/Home.tsx` now monitoring any change in a user log-in/out which will trigger a new `API.request` to get the _packages_ from the Verdaccio-server. This is done by creating a `useEffect` on **isUserLoggedIn**. Code has been transplanted from `App/App.tsx` including the use of the Loading component during the XHR. The use of **packages** was removed from other components as no longer needed and tests updated.
Test snapshots updated

Resolves issue #414
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@codecov
Copy link

codecov bot commented Jan 12, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@18d6257). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #415   +/-   ##
=========================================
  Coverage          ?   88.45%           
=========================================
  Files             ?      137           
  Lines             ?      953           
  Branches          ?      196           
=========================================
  Hits              ?      843           
  Misses            ?       99           
  Partials          ?       11

Copy link
Member Author

@coolsp coolsp left a comment

Choose a reason for hiding this comment

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

Test snapshots updated

@juanpicado juanpicado merged commit 222ffed into verdaccio:master Jan 12, 2020
@juanpicado
Copy link
Member

Thanks so much for fixing this!

juanpicado added a commit that referenced this pull request Jan 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failed to load all packages after login 2
2 participants