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

i_1042 Allow modification of contestID #1044

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

johnbrvc
Copy link
Collaborator

Description of what the PR does

Add label and text field to allow easy display and modification of contest id on the Profiles tab.

The issue mentions adding to the Settings tab on the Administrator as well. I did not do this for 2 reasons:

  1. It's silly to have 2 different ways to set the same setting
  2. The Contest Id is not part of the ContestInformation settings, which is what the Settings tab is for. Could we make a custom section for it? Yes. But I refer you back to 1).

Issue which the PR addresses

Fixes #1042
See NOTE: at bottom - this is not ready to be merged into develop until after #1040

Environment in which the PR was developed (OS,IDE, Java version, etc.)

OS : Windows 11 10.0 (amd64)
Java Version : 1.8.0_321

Precise steps for testing the PR (i.e., how to demonstrate that it works correctly)

  1. Start up any contest.
  2. Go to the Administrator->Profiles tab
  3. Note the Contest Id is now shown on the page, and you can modify it as well.

Note that is causes a Warning print in the log if you change the contest Id. The warning says the contest Id has changed in the return packet. This is true, and is not harmful. The Packet code in PC2 shows this warning when this condition happens, presumably, in case it is not intentional.

NOTE: If the contest Id is change, no checking is performed on the validity of the Contest Id supplied. The Id should be validated. I will update this PR when PR #1040 is merged since there is a routine in there that checks for the validity of a contest Id.

Add label and text field to allow easy display and modification of contest id
@johnbrvc johnbrvc added this to the 9.11.0 milestone Feb 10, 2025
@johnbrvc johnbrvc self-assigned this Feb 10, 2025
@clevengr
Copy link
Contributor

I think I disagree with your statement It's silly to have 2 different ways to set the same setting. We have situations like that throughout PC2 -- the Server and the Admin can both control the contest clock, for example.

In addition, as a user I would want to be able to find the way to change something in the place I thought "most logical". I am not at all certain that I would think of looking in "Profiles" to change the "ContestId"; I'm pretty sure I would think of that as more of a "Setting" -- which would lead me to look under the "Settings" tab.

I do grant that "ContestId" isn't (currently) part of the "ContestInformation" class -- but I'm not sure that's sufficient justification for not making it available under the "Settings" tab. Bottom line: I think this is a "user convenience/usability" issue, and I think I come down on the side of having it available in the (to me) most logical place (i.e, in the Settings tab).

However, I could live with putting this off to a separate, new, Issue...

Copy link
Contributor

@clevengr clevengr left a comment

Choose a reason for hiding this comment

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

Code changes look reasonable, and I've tested this under Windows 11. Seems good to me.

Copy link
Collaborator

@kkarakas kkarakas left a comment

Choose a reason for hiding this comment

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

LGTM

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.

Allow modification and display of Contest ID (Identifier)
3 participants