Single service mode #68

lexoyo · 2018-08-24T17:46:44Z

closes #62

it is a simplified version of #62

… is never set by unifile (my PR has never been merged)

clemos

pretty difficult to follow :|

clemos · 2018-08-25T05:54:42Z

src/js/UnifileService.js

@@ -299,6 +299,7 @@ export default class UnifileService {
  }

  static isService (file) {
-    return typeof file.isLoggedIn !== 'undefined';
+    console.log('isService', file, file.isLoggedIn);
+    return typeof file.mime === 'application/json';


weird, pls comment

the console log is a mistake

do you think this is ok for a comment?

this mime type is what the server returns when listing services... and I think it never will be the mime type of any other files

Do you think I should rather add a specific field "isService" to the data returned by the server side?

I see a lot of use case when a regular file will have a application/json MIME type, so yes, it should already be in the data without duck-typing

Ok, so do you think I should re-open this PR?
#50

I see a lot of use case when a regular file will have a application/json MIME type, so yes, it should already be in the data without duck-typing

for a static file this would be strange
if you remember we decided that this would be the mime type of services, which make them apear with an icon of service

You're thinking too much with Silex, not CE. As a developer, my FTP might be full of JSON config files I saved here.

You wouldn't want that to mess up CE

clemos · 2018-08-25T05:57:05Z

src/js/UnifileService.js

@@ -124,9 +124,9 @@ export default class UnifileService {
    });
  }

-  cd (path) {
+  cd (path, preventAuth=false) {


I think you should add it as a flag a the service level, around

CloudExplorer2/src/js/UnifileService.js

Line 56 in 958e60e

nameMap.set(service.name, service.displayName);

service map should be Map<String, {displayName, requiresAuth}>

in fact some services won't require auth, but this is about another topic here: when CE starts, if the path is "/" it will list services, and if there is only one service we want to cd into this folder. But without auth because we don't want a popup to be opened and blocked (not in a onclick). What happen here is we cd the service and if the user is not logged in it will return a 403 forbidden and we will be back to "/"

The best thing would be to send a isLoggedIn field with the services, so if the user is loggedin we cd in it, and when we cd a service to which we are logged in, CE should not bother opening a popup for auth. I made a PR for that but I think it died forgotten.. Maybe it was wrong :-/

clemos · 2018-08-25T05:57:50Z

src/js/UnifileService.js

    return new Promise((resolve, reject) => {
-      if (path.length === 1 && path[0] !== this.currentPath[0]) {
+      if (!preventAuth && path.length === 1 && path[0] !== this.currentPath[0]) {


this check is weird (it was weird already)

yeah.. it's a way to check if we are opening a service or a folder... we should probably use isService or something
but we don't have the files yet at this level, only the path
any suggestions?

We could retrieve the service with the path, right? If it's the first part of the path, use the service Map to get the service and fetch the service auth strategy (I think that's what @clemos was trying to say)

ok, then that's what path.length === 1 does

clemos · 2018-08-25T06:01:34Z

src/js/CloudExplorer.jsx

@@ -162,6 +162,15 @@ export default class CloudExplorer extends React.Component {
            files,
            loading: false
          });
+          // the first display, single service mode, try to enter
+          // this is useful when CE is used by hosting companies to display the user files, and the user has logged in their system
+          if(!this.initDone && files.length === 1 && path.length === 0/* && files[0].isLoggedIn === true*/) {


I think initDone should be part of the component state.

but more generally, I don't quite understand this part.

yes again that's a way to open a service when CE starts, so only the 1st time and if it is "/" which is ls
maybe i should put all this somewhere else...

JbIPS

I agree with @clemos

JbIPS · 2018-08-25T20:03:46Z

src/js/UnifileService.js

    return new Promise((resolve, reject) => {
-      if (path.length === 1 && path[0] !== this.currentPath[0]) {
+      if (!preventAuth && path.length === 1 && path[0] !== this.currentPath[0]) {


We could retrieve the service with the path, right? If it's the first part of the path, use the service Map to get the service and fetch the service auth strategy (I think that's what @clemos was trying to say)

JbIPS · 2018-08-25T20:05:31Z

src/js/UnifileService.js

@@ -299,6 +299,7 @@ export default class UnifileService {
  }

  static isService (file) {
-    return typeof file.isLoggedIn !== 'undefined';
+    console.log('isService', file, file.isLoggedIn);
+    return typeof file.mime === 'application/json';


I see a lot of use case when a regular file will have a application/json MIME type, so yes, it should already be in the data without duck-typing

lexoyo · 2018-08-26T09:05:10Z

Ok so

I have fixed the issues you raised
I have added a "isLoggedIn" info and "isService" info as I previously wanted to do in the old PR
since this prevented any call to Dropbox or Github when the user had been logged in once, I had to add a logout button - which I did at the service level in the UI, but a "logout all" button somewhere might be needed by @Godzone for the single service use case

lexoyo · 2018-08-26T09:08:44Z

src/js/CloudExplorer.jsx

lexoyo · 2018-08-31T17:44:59Z

oh there is an old issue about the linter: #45

lexoyo · 2018-09-01T19:38:37Z

+1 Le 1 sept. 2018 à 14:10, à 14:10, "Clément Charmet" <[email protected]> a écrit:

…

clemos commented on this pull request. > @@ -162,6 +162,18 @@ export default class CloudExplorer extends React.Component { files, loading: false }); + // the first display, single service mode, try to enter + // this is useful when CE is used by hosting companies to display the user files, and the user has logged in their system + if(!this.initDone && files.length === 1 && Oh, and also: fuck pre-commit hooks !!!!! -- You are receiving this because you modified the open/close state. Reply to this email directly or view it on GitHub: #68 (comment)

lexoyo added 3 commits August 24, 2018 19:41

fixed dropbox default callback route

58e90c7

fixed detect a unifile service thx to isLoggedIn since this attribute…

20815d5

… is never set by unifile (my PR has never been merged)

single service mode closes silexlabs#62

958e60e

lexoyo mentioned this pull request Aug 24, 2018

Single service mode #62

Closed

lexoyo requested review from JbIPS and clemos August 24, 2018 17:49

clemos requested changes Aug 25, 2018

View reviewed changes

JbIPS requested changes Aug 25, 2018

View reviewed changes

added isLoggedIn info returned by server for services, added logout

369e97a

lexoyo force-pushed the master branch from 6a5218a to 369e97a Compare August 26, 2018 09:02

clemos requested changes Aug 27, 2018

View reviewed changes

src/js/CloudExplorer.jsx Outdated Show resolved Hide resolved

lexoyo added 3 commits August 31, 2018 19:04

initDone flag in the state of CloudExplorer component

f378ba5

initDone flag in the state of CloudExplorer component

21f1e66

fixed some linter errors

c559016

lexoyo merged commit 2ebea1a into silexlabs:master Aug 31, 2018

This was referenced Aug 31, 2018

Do not re-auth when the user is already logged in #49

Closed

WIP: More info in sercices route #50

Closed

Skip auth for FS silexlabs/unifile#141

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Single service mode #68

Single service mode #68

lexoyo commented Aug 24, 2018

clemos left a comment

clemos Aug 25, 2018

lexoyo Aug 25, 2018

JbIPS Aug 25, 2018

lexoyo Aug 26, 2018

lexoyo Aug 26, 2018

JbIPS Aug 26, 2018

clemos Aug 25, 2018

lexoyo Aug 25, 2018

clemos Aug 25, 2018

lexoyo Aug 25, 2018

JbIPS Aug 25, 2018

lexoyo Aug 26, 2018

clemos Aug 25, 2018

clemos Aug 25, 2018

lexoyo Aug 25, 2018

JbIPS left a comment

JbIPS Aug 25, 2018

JbIPS Aug 25, 2018

lexoyo commented Aug 26, 2018

lexoyo commented Aug 26, 2018

lexoyo commented Aug 31, 2018

lexoyo commented Sep 1, 2018 via email

Single service mode #68

Single service mode #68

Conversation

lexoyo commented Aug 24, 2018

clemos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JbIPS left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lexoyo commented Aug 26, 2018

lexoyo commented Aug 26, 2018

lexoyo commented Aug 31, 2018

lexoyo commented Sep 1, 2018 via email