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

Refactor Session and Session_iba template #907

Closed
8 tasks
eleanorreem opened this issue May 15, 2024 · 7 comments · Fixed by #1181
Closed
8 tasks

Refactor Session and Session_iba template #907

eleanorreem opened this issue May 15, 2024 · 7 comments · Fixed by #1181
Assignees
Labels
complexity: advanced Time needed to do this ticket will be large e.g. 2-3+ days hacktoberfest Hacktoberfest issues help wanted Extra attention is needed maintenance Maintenance / chore work priority: 1+week Should be prioritized next week or longer. state: approved Ready to go. Not blocked or pending.

Comments

@eleanorreem
Copy link
Contributor

eleanorreem commented May 15, 2024

Overview

We created a duplicate but slightly different template for our IBA course. There is now a large amount of shared loginc between the regular session template and the session_iba template.

The IBA session page can be found in pages/courses/image-based-abuse/[sessionSlug].tsx and the regular session page can be found in pages/courses/[slug]/[sessionSlug].tsx

This ticket is to pull out duplicated logic / code between the two pages into either separate components or functions that can then be shared by the two pages.

Action Items

Here are some suggestions for possible refactors. These are suggestions and you may take a different route:

  • Extract logic which determines chat access into separate function
  • Extract logic which determines session completion into separate function
  • Create SessionHeader component which wraps around the Header component and its children
  • Create SessionVideo component to wrap around current implementation with SessionContentCard which contains Video and transcript as children
    • callStartSession can move into this component
    • the useEffect to open transcript modal can also be moved to this component
  • to handle Create Activity component to wrap around current implementation with SessionContentCard
  • Create Chat component to wrap around the current implementation of the Chat card

Testing

  • Once your refactor is complete, please compare your pages to the live pages.
  • Ensure you have run cypress tests and everything still works

It may make sense to split this ticket out into separate PRs as there could be a lot of changes.

Resources/Instructions

@eleanorreem eleanorreem added help wanted Extra attention is needed complexity: advanced Time needed to do this ticket will be large e.g. 2-3+ days maintenance Maintenance / chore work state: approved Ready to go. Not blocked or pending. priority: 1+week Should be prioritized next week or longer. labels May 15, 2024
@anmol-fzr
Copy link
Contributor

@eleanorreem I would like to contribute on this issue please assign me this issue

@kyleecodes
Copy link
Member

Hey @anmol-fzr thank you for your interest!
I can assign you after this PR is either closed or completed.
As per our Issue Limit policies sometimes we have to limit the number of issues assigned to help keep a sustainable flow of issues for all contributors. Thank you!

@MarosBaran
Copy link
Contributor

@kyleecodes Hi, I am a student of Technical University of Kosice (Slovakia) and i would like to contribute on this issue as part of a task i was given on one of my subjects (i can provide details about this task in dm/pm). Can u assign me this issue if its still available?

Copy link
Contributor

Thank you @MarosBaran you have been assigned this issue!
Please follow the directions in our Contributing Guide. We look forward to reviewing your pull request shortly ✨


Support Chayn's mission? ⭐ Please star this repo to help us find more contributors like you!
Learn more about Chayn here and explore our projects. 🌸

@MarosBaran
Copy link
Contributor

Hi @kyleecodes , im already working on this Issue, ive added the SessionVideo/SessionHeader components, extracted the logic for session completion, but now i have 1 question. In StoryBlokSessionPage and StoryBlokSessionIbaPage, there's a different logic for liveChatAccess, in StoryBlokSessionPage it takes into account the liveCourseAccess as well, but it doesnt in StoryBlokSessionIbaPage. Now my question is if thats intended or that it should be fixed to so that both pages share the same logic. I provide screenshots of mentioned code, thank you for your answer.
Screenshot 2024-10-20 at 13 54 32
Screenshot 2024-10-20 at 13 54 23

@kyleecodes
Copy link
Member

@eleanorreem

In StoryBlokSessionPage and StoryBlokSessionIbaPage, there's a different logic for liveChatAccess, in StoryBlokSessionPage it takes into account the liveCourseAccess as well, but it doesnt in StoryBlokSessionIbaPage. Now my question is if thats intended or that it should be fixed

Pinging @eleanorreem !

@kyleecodes kyleecodes added the hacktoberfest Hacktoberfest issues label Oct 22, 2024
@annarhughes
Copy link
Member

Hey @MarosBaran thanks for checking in on this liveAccess issue! Coincidentally I had edited this logic so that both files use the same logic. The changes are here and will be merged next week, so we may have a slight but please do continue refactoring, with both files using the same logic as you had intended to. We'll replace my change with your refactors later 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity: advanced Time needed to do this ticket will be large e.g. 2-3+ days hacktoberfest Hacktoberfest issues help wanted Extra attention is needed maintenance Maintenance / chore work priority: 1+week Should be prioritized next week or longer. state: approved Ready to go. Not blocked or pending.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants