Hi Dushan, First, great work on implementing the multi-tenancy support to Store SPA, This improvement will allow users to directly access a tenant store,share tenant store URL etc. I have a few suggestions on the implementation.
- We already have a query string parser/formatter library *qs <https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/package.json#L42> *shall we use that library in here[1 <https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L52> ][2 <https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L56>] too ? is there any limitations with that library ? - Shall we move the BrowserRouter.jsx file to */store-new/source/src/app/components/Base/* directory, Since this file is a React component, it should come under components directory and there we could move it to Base components directory? - Shall we add the props validation in the original BrowserRouter <https://github.com/ReactTraining/react-router/blob/master/packages/react-router-dom/modules/BrowserRouter.js#L19-L25>[4] implementation? - And also I think we now need to keep an eye on the original (react-router-dom) BrowserRouter.js <https://github.com/ReactTraining/react-router/blob/master/packages/react-router-dom/modules/BrowserRouter.js#L19-L25> implementation, To absorb their changes to our component. [1]: https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L52 [2]: https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L56 [3]: https://github.com/dushansilva/carbon-apimgt/tree/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/components/Base [4]: https://github.com/ReactTraining/react-router/blob/master/packages/react-router-dom/modules/BrowserRouter.js#L19-L25 Thanks ~KasunTe On Thu, Sep 5, 2019 at 3:29 PM Dushan Silva <[email protected]> wrote: > Hi all, > I have been working on $Subject. While working on it we came across the > following requirement. > > In the previous store implementation the tenant domain is passed on the > url as a query param and when either the page is refresh or if the url is > directly accessed with the query param tenant specific details are loaded. > However in the current react based single page implementation all the > routing is handle via react router. Here if we initially plan on keeping > the tenant domain in the react context api as a global setting. Once the > app is loaded routing can be handled easily by taking the tenant domain > from the context api. > > The problem is when the app is refreshed or if a url is visited directly, > at this occasion the context api is empty and the user will always be > redirect to the tenant listing. To handle this issue after considering some > input from the react community [1] , we came up with the solution of > writing an interceptor for the react router. This would require us to > extend the currently react router implementation and its history > implementation to plugin our custom interceptor. An advantage of the > interceptor approach is that this saves the developer the hassle handling > both tenant and normal cases each and every route path already added and > paths to be added in the future. The flow will be as follows. > > When the store is initially loaded react router should be initialized [2]. > Rather than using the default Router component by react router we will be > using our custom component BrowserRouter [3]. In this we place the > interceptor to be invoked when we are navigating around (push or replace) > [4]. The interceptor [5] will take the tenant domain from the context and > append it to the path. So in each route (when the user navigates to > different pages this will be appended as a query param. > > To retrieve the tenant specific data from the REST API , we will be > getting the tenant domain from the query param and setting it to the > x-wso2-tenant header [6]. > > Any thoughts and feedback on this much appreciated. > > > [1] - > https://stackoverflow.com/questions/57757263/how-to-write-an-interceptor-for-react-router/57757921#57757921 > [2] - > https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/App.jsx#L96 > [3] - > https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L15 > [4] - > https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L28 > [5] - > https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L43 > [6] - > https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/data/APIClient.jsx#L163 > -- > Best Regards > Dushan Silva > Software Engineer > > *WSO2, Inc. * > > lean . enterprise . middleware > Mob: +94 774 979042 > -- *Kasun Thennakoon* | Senior Software Engineer | WSO2 Inc. (m) +94 711661919 | (w) +94 11 214 5345 | (e) [email protected] GET INTEGRATION AGILE Integration Agility for Digitally Driven Business
_______________________________________________ Architecture mailing list [email protected] https://mail.wso2.org/cgi-bin/mailman/listinfo/architecture
