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

fix: Link sessions in speaker list to sessions page #4853

Merged
merged 5 commits into from Sep 2, 2020
Merged

fix: Link sessions in speaker list to sessions page #4853

merged 5 commits into from Sep 2, 2020

Conversation

maze-runnar
Copy link
Contributor

Fixes #4832

Checklist

  • I have read the Contribution & Best practices Guide.
  • My branch is up-to-date with the Upstream development branch.
  • The acceptance, integration, unit tests and linter pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@vercel
Copy link

vercel bot commented Aug 24, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/eventyay/open-event-frontend/das84sbio
✅ Preview: https://open-event-frontend-git-fork-maze-runnar-patch-2.eventyay.vercel.app

@maze-runnar
Copy link
Contributor Author

captured (1)

@codecov
Copy link

codecov bot commented Aug 24, 2020

Codecov Report

Merging #4853 into development will increase coverage by 0.30%.
The diff coverage is 0.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #4853      +/-   ##
===============================================
+ Coverage        22.85%   23.16%   +0.30%     
===============================================
  Files              484      481       -3     
  Lines             5127     5099      -28     
  Branches            18       18              
===============================================
+ Hits              1172     1181       +9     
+ Misses            3951     3914      -37     
  Partials             4        4              
Impacted Files Coverage Δ
app/controllers/public/session/view.js 0.00% <0.00%> (ø)
app/router.js 100.00% <0.00%> (ø)
app/mixins/event-wizard.js 1.13% <0.00%> (ø)
app/helpers/ui-grid-number.js 34.78% <0.00%> (ø)
app/components/forms/wizard/attendee-step.js 0.00% <0.00%> (ø)
app/routes/public/sessions-index.js
app/controllers/events/view/edit/other-details.js
app/components/forms/wizard/other-details-step.js
app/components/forms/wizard/basic-details-step.js 24.39% <0.00%> (+5.81%) ⬆️
app/utils/computed-helpers.js 30.00% <0.00%> (+8.04%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7a4b41...ca2392c. Read the comment docs.

@iamareebjamal
Copy link
Member

That's the page we want to show only to the speaker. The session page linked should show the expanded session tile on the page

@iamareebjamal
Copy link
Member

So, /session/:id will be page to show to speakers (not public)

And /sessions/:id should be public session page with SessionItem component expanded.

@iamareebjamal
Copy link
Member

Also, someone who's not a speaker shouldn't be able to see the withdraw and edit session buttons

@maze-runnar
Copy link
Contributor Author

Also, someone who's not a speaker shouldn't be able to see the withdraw and edit session buttons

could it be that everyone can see that page but disable button, if user is not authorized to withdraw and edit session?
is there any this kind of feature is available?

@iamareebjamal
Copy link
Member

Buttons should be hidden but everyone can see the page

@maze-runnar
Copy link
Contributor Author

@iamareebjamal what about this #4832 (comment) ?

@maze-runnar
Copy link
Contributor Author

maze-runnar commented Aug 27, 2020

is it an correct implementation if only one of speakers can edit http://localhost:4200/e/16fa59c7/session/6240 page, but every one can see?

@maze-runnar
Copy link
Contributor Author

Session Detail page when one is not authorized to edit details and withdraw proposal -
Screenshot from 2020-08-27 22-48-50
Session Detail page when one is authorized to edit details and withdraw proposal -
Screenshot from 2020-08-27 22-48-43

@mariobehling
Copy link
Member

This is a single session view, right? So, there is no way to filter a session here and therefore no need to show track and room filter in the sidebar.

@iamareebjamal
Copy link
Member

That's when I didn't remove the sidebar from other pages. @maze-runnar must not have rebased the latest changes, thus the screenshot

@iamareebjamal iamareebjamal merged commit f0b0571 into fossasia:development Sep 2, 2020
@maze-runnar maze-runnar deleted the patch-2 branch September 2, 2020 06:07
@mariobehling
Copy link
Member

The implementation does not work for anonymous users. The linked page should be available openly on the web (without options to edit session details). Please give permissions to view this page for everyone. Therefore opening the issue.

Compare event here: https://eventyay.com/e/8fa7fd14/speakers and click on sessions without being logged in.

@iamareebjamal
Copy link
Member

Opened a new issue for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Link sessions in speaker list to sessions page
3 participants