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

Bug : Incorrect Portfolio link #155

Open
Rahulm2310 opened this issue Jan 22, 2021 · 35 comments
Open

Bug : Incorrect Portfolio link #155

Rahulm2310 opened this issue Jan 22, 2021 · 35 comments
Assignees
Labels
Category: Coding Changes to code base or refactored code that doesn't fix a bug. Status: On Hold Issue or PR needs more info, a discussion, a review or approval from a Maintainer/Code Owner. Type: Bug Bug or Bug fixes.

Comments

@Rahulm2310
Copy link
Contributor

Describe the bug
When visiting a member's profile, if we click on "Go to Portfolio" button (link) , it's href is set with a incorrectly formatted member portfolio url.

To Reproduce
Steps to reproduce the behavior:

  1. Go to a member's profile
  2. Scroll down
  3. Hover on "Go to Portfolio" link
  4. Check it's href attribute

Expected behavior
It should be correctly formatted.

Screenshots
portfolio-issue

The button currently goes to http://localhost:3000/members/profile/"/members/portfolio/rmohata" whereas it should go to http://localhost:3000/members/portfolio/rmohata

Desktop (please complete the following information):

  • OS: Windows 10
  • Browser: Chrome
  • Version : 87.0.4280.141

I would like to take up and fix this issue.

@Rahulm2310
Copy link
Contributor Author

@vj-codes @Aaishpra could you guys please confirm this issue ?

@Aaishpra
Copy link
Member

@Rahulm2310 can you confirm if the URL is working or not means does it takes you to the right place, cuz in the code I see
image
image
and removing extra / and double quotes from start and end is possible but not profile meaning the correct pattern can be http://localhost:3000/members/profile/members/portfolio/rmohata.
Although this issue is valid if the current URL is not working.
Also @Rahulm2310 can you check if the URL I provided here works and takes you to the right place.

@Rahulm2310
Copy link
Contributor Author

Rahulm2310 commented Jan 27, 2021

@Aaishpra Currently the URL is contains "" quotes i.e. the url is not correctly formatted. It is not working currently. Also even if we remove quotes, http://localhost:3000/members/profile/members/portfolio/rmohata it is still seems to be a invalid url. Still I will check and confirm. I think http://localhost:3000/members/portfolio/rmohata should be the correct format.

@Rahulm2310
Copy link
Contributor Author

Rahulm2310 commented Jan 27, 2021

@Aaishpra both the links get redirected to home page.

image
Although, this shows that http://localhost:3000/members/portfolio/rmohata should be the valid url.

@Aaishpra
Copy link
Member

Aaishpra commented Jan 28, 2021

Oh I see, Thanks for pointing it out 🤔. Although the member portfolio page is still under construction but the URL should work.

@mtreacy002 what do you think about this?

@mtreacy002
Copy link
Member

@Rahulm2310 and @Aaishpra . Ideally, we would like to have user (e.g. user A) to see the portfolio page of another member (e.g. User B), not the profile page, when user A click on that user B's name from the members list (on the Members page). So, the route for this page will be http://localhost:3000/meembers/portfolio/userb. Then on B's Portfolio page, we'll place a link to the B's Profile page so A can see B's details (this topic is open for discussion, I prefer if users' details is kept only to the individual it belongs to, not other members). In this case, the route we should see is http://localhost:3000/members/portfolio/userb/profile
However, since we're yet to touch Portfolio page, atm we temporarily direct users to the member Profile page instead.
We can create a template for user's Portfolio page so we can start using the proper routing. What do you all think?

@Rahulm2310
Copy link
Contributor Author

Then on B's Portfolio page, we'll place a link to the B's Profile page so A can see B's details (this topic is open for discussion, I prefer if users' details is kept only to the individual it belongs to, not other members). In this case, the route we should see is http://localhost:3000/members/portfolio/userb/profile

@mtreacy002 If I am not wrong, isn't there a route like http://localhost:3000/members/profile/userb for visiting a user' s profile.

We can create a template for user's Portfolio page so we can start using the proper routing. What do you all think?

Yes, I agree.

@Aaishpra
Copy link
Member

Completely agree with you on the routing part. @mtreacy002
Also i suggest first we can create a template for member portfolio and then when we have the template ready someone can work on the implementation part along with working on this issue for proper routing of it.
So, until a template is ready this issue should be put to hold.

Cc @mtreacy002

@mtreacy002
Copy link
Member

mtreacy002 commented Jan 29, 2021

@mtreacy002 If I am not wrong, isn't there a route like http://localhost:3000/members/profile/userb for visiting a user' s profile.

@Rahulm2310 , the route you're referring to is only a temporary route since we don't have the portfolio page ready yet. We currently do have MemberPortfolio component for this (Portfolio page), but it hasn't been utilised since it doesn't have the minimum information to accept/pass props to the next page. We need to construct this MemberPortfolio so we can direct user from the Members list (Members page) to the Portfolio's page (which what should happen - our goal). Then from there (Portfolio page), we can direct user to the Profile page.

That means, once the MemberPortfolio component ready with this basic function, we will have the following routings:

  • for MemberPortfolio component, the path will remain the same - "/members/portfolio/".
    So if user A select user B from Members list (Members page), they'll be redirected to this url - http://localhost:3000/members/portfolio/userb.
    from this portfolio page, user A can click a button to go to user B's Profile page.
  • for Member component, the path will be changed to just "/profile/"
    When user A who's clicked that button 'Profile' above to see user B's profile, they will be redirected to this url - http://localhost:3000/members/portfolio/userb/profile.
    On this page (Profile) we can give user A options to select which of user B's details they want to see (Personal details, Additional info, or Personal background - for privacy reason, perhaps we should keep this feature for Admin user only).
    Now, let's say user A chose to view user B's personal details, then on button click they'll be redirected to the following url - http://localhost:3000/members/portfolio/userb/profile/personal-details

So, I agree with @Aaishpra comment, ...

Also i suggest first we can create a template for member portfolio and then when we have the template ready someone can work on the implementation part along with working on this issue for proper routing of it.
So, until a template is ready this issue should be put to hold.

... we can put this issue on hold and open an issue for making the Portfolio page (MemberPortfolio component) ready then revisit this issue with a cleaner url path.

What do you all think?

@Aaishpra
Copy link
Member

+1 from my side @mtreacy002 the portfolio needs to be created first.

@Rahulm2310
Copy link
Contributor Author

Ok, then let's first create the Portfolio page and then get back to this 👍

@mtreacy002 mtreacy002 added Category: Coding Changes to code base or refactored code that doesn't fix a bug. Status: On Hold Issue or PR needs more info, a discussion, a review or approval from a Maintainer/Code Owner. Type: Bug Bug or Bug fixes. labels Feb 24, 2021
@mtreacy002
Copy link
Member

@Rahulm2310, I've rethink this over, and feel like we can just use the existing Portfolio template (component) for this purpose.
But we need to make sure that the url http://localhost:3000/members/portfolio/username is accessible through both Members > username and My Space > Portfolio

Screen Shot 2021-03-04 at 10 14 31 pm

At the moment, when user click Members > username, they'll be directed to Profile page, so we can change this to the existing Portfolio template, showing the selected member instead of Member's page (can be done through passing prop). Remember to use .../username at the end of the url.
Screen Shot 2021-03-04 at 10 15 43 pm

I'll make this issue available now.

@mtreacy002 mtreacy002 added Status: Available Issue was approved and available to claim or abandoned for over 3 days. and removed Status: On Hold Issue or PR needs more info, a discussion, a review or approval from a Maintainer/Code Owner. labels Mar 4, 2021
@Rahulm2310
Copy link
Contributor Author

Rahulm2310 commented Mar 4, 2021

@mtreacy002 Thanks! I would like to pick it up. Please assign me 🙂

showing the selected member instead of Member's page

What should be shown in place of Member's when user visits My Space > Portfolio (user's own portfolio) ?

@mtreacy002 mtreacy002 removed the Status: Available Issue was approved and available to claim or abandoned for over 3 days. label Mar 4, 2021
@mtreacy002
Copy link
Member

mtreacy002 commented Mar 4, 2021

What should be shown in place of Member's when user visits My Space > Portfolio (user's own portfolio) ?

It needs to show the name of the user, e.g. Rahul's Portfolio. Coz it doesn't matter if it's user's own or other's portfolio, we can display the name as the user name (no need for My Portfolio. This style is used on many websites (LinkedIn, Github, Facebook, etc)

@Rahulm2310
Copy link
Contributor Author

@mtreacy002 agreed. Thanks for clarifying 😊

@Rahulm2310
Copy link
Contributor Author

@mtreacy002 I see two components namely MemberPortfolio.jsx and Portfolio.jsx with same content. Which component is to be used here?

@mtreacy002
Copy link
Member

mtreacy002 commented Mar 5, 2021

@Rahulm2310 , in that case you can direct the call from 'Members' to 'MemberPortfolio' and the one from 'My Space' to 'Portfolio'. Remember to give relevant buttons/links on this templates for the next actions, e.g. :

  • a button for Organization Representative to send request for them to work on a program (only enabled to a user with Organization) on 'MemberPortfolio'
  • a button to go to the 'Profile' page on both templates

Later once we completed the feature for creating/accepting mentoring program relation, we can put the programs that portfolio owner had worked/currently working on as active links.
Let me know what you think of this 😉
PS: I initially thought to just use on of the templates for both endpoints, but concern about the complexity of the logic and flows of action for links/buttons. So, let's keep it simple (separate) for now, which is also consistent with how we treat other components under 'Profile'. Later we can see what we can do/if there's a need to refactor.

@Rahulm2310
Copy link
Contributor Author

Rahulm2310 commented Mar 6, 2021

a button to go to the 'Profile' page on both templates

@mtreacy002 Should a user be allowed to view another user's personal details ?
Also, should the button Go to Profile page go to http://localhost:3000/members/profile/username on both the pages (MemberPortfolio and Portfolio) as we don't have a /profile route for logged in user

a button for Organization Representative to send request for them to work on a program (only enabled to a user with Organization) on 'MemberPortfolio'

How can I check if a user is a org representative ? By fetching /organization endpoint ?

@mtreacy002
Copy link
Member

mtreacy002 commented Mar 7, 2021

@mtreacy002 Should a user be allowed to view another user's personal details ?

@Rahulm2310, The BridgeInTech Admin user can see user's personal information, but for other users, only those who are an organization representative can. This is because they might need to contact potential mentor/mentee /via email/phone, check out the organization that user is working at, check the personal background of that user if they have a program targeting specific background, etc. Normal users should not have access to these info.

Also, should the button Go to Profile page go to http://localhost:3000/members/profile/username on both the pages (MemberPortfolio and Portfolio) as we don't have a /profile route for logged in user

1st point:
At the moment that url renders that user's personal details page. For this PR, we have 2 options to show this user's Profile page:

  1. Create a Profile page that shows 3 available pages as the next actions:
    • to user's personal details
    • to user's additional information
    • to user's personal background
  2. directly direct the url to that user's personal details when the button Go to Profile is clicked. Then on that Personal details page we can place buttons that will direct user to either the Additional Information page or the Personal Background page.

IMO, both approach will have its plus and minus, but if have to choose, I'd go with option 1.

2nd point:
To consider whether or not the route http://localhost:3000/members/profile/username can be for both MentorPortfolio and Portfolio component (which means using the same Profile component), we should look at the complexity it might cause just as we considered the impact to the Portfolio page above.
There won't be other actions possible from logged in user who views other users's profile. Even for user who views their own profile page, they only need to be able to update their details if they need to (via Edit button). So, the Portfolio component won't have as complex code as the MemberPortfolio and Portfolio pages (summary of all mentoring projects, achievements, feedbacks, events, etc)
With the consideration above, II would say we could use the route http://localhost:3000/members/profile/username for both MentorPortfolio and Portfolio component.

How can I check if a user is a org representative ? By fetching /organization endpoint ?

Yes, for this one you need to send the GET /organization call to BIT backend. Tips: You might have to consider how to keep the info returned by this call persistent for the logged in user as few pages will require this info (e.g. organisation rep has different level of permission to other user's personal info as I mentioned at the beginning of this post). So, making it persistent will help remove the need for making the same request again and again.
Screen Shot 2021-03-07 at 12 47 13 pm

@Rahulm2310
Copy link
Contributor Author

Create a Profile page that shows 3 available pages as the next actions:
to user's personal details
to user's additional information
to user's personal background

@mtreacy002 In case when a logged in user visits their profile, We can show buttons on the Profile page for going to Personal Details, Additonal Information and Personal Background . But what happens when a visitor (not a org representative) visits a user's profile. As we can't show personal details of a user to the visitor (until he/she is a org representative), then what should be shown ?

With the consideration above, II would say we could use the route http://localhost:3000/members/profile/username for both MentorPortfolio and Portfolio component.

Actually, the url looks weird to a person visiting his own profile. We can use the same component to render, but should use different routes like for logged in user's own profile, it should be /profile and for visiting other user's profile it can be /members/profile/username.

You might have to consider how to keep the info returned by this call persistent for the logged in user as few pages will require this info (e.g. organisation rep has different level of permission to other user's personal info as I mentioned at the beginning of this post). So, making it persistent will help remove the need for making the same request again and again.

For this, I was thinking of creating a context similar to AuthContext and persist the data using cookies.

@mtreacy002
Copy link
Member

@mtreacy002 In case when a logged in user visits their profile, We can show buttons on the Profile page for going to Personal Details, Additonal Information and Personal Background . But what happens when a visitor (not a org representative) visits a user's profile. As we can't show personal details of a user to the visitor (until he/she is a org representative), then what should be shown ?

The logic for the button "Go to Portfolio" on the MemberPortfolio page should check the user identity and role:

  • if user not representing the organization and not the owner of the personal data, disable the button
  • regardless of whether the user is organization representative or not, if they are the owner of the personal data, enable the button
  • if that user is an organization representative but not the owner of the personal data, enable button so this user can view the member's details (just make sure those pages Edit button is hidden for this user).

Actually, the url looks weird to a person visiting his own profile. We can use the same component to render, but should use different routes like for logged in user's own profile, it should be /profile and for visiting other user's profile it can be /members/profile/username.

Makes sense. Ok, let's go with that 👍

For this, I was thinking of creating a context similar to AuthContext and persist the data using cookies.

perhaps we can use the existing AuthContext and make the call to GET /organization on the initial login logic. This way no need to create a separate one?

@Rahulm2310
Copy link
Contributor Author

perhaps we can use the existing AuthContext and make the call to GET /organization on the initial login logic. This way no need to create a separate one?

@mtreacy002 Sure, for now we don't need a seperate context for organization. Will save the org data in the existing AuthContext. Thus no need to make fetch calls for it again. 👍

One more thing is that, we have routes /personal-details , /additional-information, and /personal-background for logged in user visiting their details. When a org representative visits user's personal-details the route should be like /members/profile/username/personal-details. Similarly for additional-details and personal-background.

Also, currently we are showing input forms in /personal-details, /personal-details , /additional-information, and /personal-background like this :
image
image

We need not to show these inputs and directly show the values when a representative visit's user's personal-details. For that either we need to create a seperate page or render the form conditionally based on if user visiting his own profile or representative visiting a candidate's profile. Does this makes sense ?

@mtreacy002
Copy link
Member

@Rahulm2310 the logic above totally make sense 👍 . Please do go ahead with it.

@Rahulm2310
Copy link
Contributor Author

@mtreacy002 As we send access_token to the backend to fetch personal-details, additional-information, personal background, we can only get logged in user's details. I think we don't have routes setup in the backend for a representative requesting a user's additional-information, personal background. Do we ?

@mtreacy002
Copy link
Member

mtreacy002 commented Mar 10, 2021

@Rahulm2310 , yes, the request to additional info and personal background should be made to BIT endpoint 😉. Only the personal details need to be sent to MS backend.

@Rahulm2310
Copy link
Contributor Author

Rahulm2310 commented Mar 11, 2021

@mtreacy002 I meant that currently we can request personal-details, additional-information, personal-background for current user (using access_token). How can someone else (an org representative) another user's personal-details, additional-information, personal-background.
BIt-endpoint

Don't we need to create endpoints for that at backend somewhat like users/{userId}/personal-details, similarly for additional-information, personal-background.

@mtreacy002
Copy link
Member

I see your point now, @Rahulm2310. Yes, agree. We should create endpoints for that just as you stated above. Can you please open 2 separate issues on those endpoints? I Really appreciate you brought this up.

@Rahulm2310
Copy link
Contributor Author

@mtreacy002 Sure, will do that 👍

@Rahulm2310
Copy link
Contributor Author

@mtreacy002 Should we put this on hold until the endpoints gets created at backend.

@mtreacy002
Copy link
Member

Ok. Will do that 😉

@mtreacy002 mtreacy002 added the Status: On Hold Issue or PR needs more info, a discussion, a review or approval from a Maintainer/Code Owner. label Mar 12, 2021
@naveen8801
Copy link
Contributor

hi @mtreacy002 , is this issue open ?

@decon-harsh
Copy link
Member

hi @mtreacy002 , is this issue open ?

It's still on hold , if you wish to propose a way to solve this, write it here.

cc @mtreacy002

@Rahulm2310
Copy link
Contributor Author

Hi @naveen8801, we need the endpoints GET /users/{userId}/additional-info and GET /users/{userId}/personal-background created on the backend to work on this issue. There are open issues for these on the backend (#215 and #216). Till then we need to put this issue on hold.

@mtreacy002
Copy link
Member

@naveen8801 , the issue #215 that @Rahulm2310 mentioned above is still waiting for contributors to claim it. Would you be interested to wok on it?

@naveen8801
Copy link
Contributor

@naveen8801 , the issue #215 that @Rahulm2310 mentioned above is still waiting for contributors to claim it. Would you be interested to wok on it?

sure @mtreacy002 ,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Coding Changes to code base or refactored code that doesn't fix a bug. Status: On Hold Issue or PR needs more info, a discussion, a review or approval from a Maintainer/Code Owner. Type: Bug Bug or Bug fixes.
Projects
None yet
Development

No branches or pull requests

5 participants