Skip to content

SCRUM-61 implement conduct debate button#17

Open
Peres270 wants to merge 1 commit intomasterfrom
SCRUM-61-Implement-conduct-debate-button
Open

SCRUM-61 implement conduct debate button#17
Peres270 wants to merge 1 commit intomasterfrom
SCRUM-61-Implement-conduct-debate-button

Conversation

@Peres270
Copy link
Copy Markdown
Contributor

SCRUM-61 – Implement conduct debate button

Implemented the "Conduct Debate" button on the debate details page.

What was done
Added a new debate details page (/debates/[debate_id])
Created MarshalPanel component to display marshal actions
Integrated data fetching:
/auth/me to get current user
/tournaments/{id}/debates/{debate_id} to fetch debate details
/users/{id}/tournaments/{id}/roles to determine user roles
Implemented conditional rendering:
Button is only visible for users with Marshal or Organizer roles
Added external link to DebateCore with encoded motion

Notes / Limitations
Could not fully validate the feature end-to-end because the backend currently returns no debates (GET /tournaments/{id}/debates returns an empty array)
As a result, it was not possible to access a real debate details page to visually confirm the button rendering

The implementation is complete from the frontend side and should work correctly once a valid debate is available in the backend.

Copy link
Copy Markdown
Member

@Mateusz-Dobrzynski Mateusz-Dobrzynski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm, the /tournaments/{tournament_id}/debates/{debate_id} endpoint returns 500; I'm creating a task for it. However, you do not need this endpoint to complete the task. You only need to use /users/{user_id}/tournaments/{tournament_id}/roles to check user roles and this endpoint works just fine (refer to the testing section).

There are some problems with this PR.

  1. The component does not look the way it should:
Image

Original design

Image

Current implementation

  1. Problems with the code need to be resolved.
  • Pass motion to the component and remove other props.
  • Write unit tests for the component:
    • Mock /users/{user_id}/tournaments/{tournament_id}/roles and check if the component is rendered/not rendered depending on whether the user is a Marshal or not.
    • Ensure the button contains a valid link when you pass a motion to it.
  • Change component styling to match the design.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again: do not translate Polish strings; it will be done later by a native speaker.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could have skipped creating this file, though I understand it might have been created due to a misunderstanding about the scope of the task that we had.

Comment on lines +87 to +89
const conductDebateHref = `https://tools.debateco.re/oxford-debate/setup?motion=${encodeURIComponent(
motion
)}`;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you create this variable in the outer scope? It can be defined inside the component.

Comment on lines +7 to +11
export function MarshalPanel({
title,
buttonLabel,
href,
}: MarshalPanelProps) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you make the props generic so that all logic must be handled outside of the component? Instead, you can only pass motion and generate the href here. Displayed strings can be handled as static translations, as they will not change. Refer to the design.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants