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

github manual edit warning message disappears after editing/submitting in tester response phase #1021

Open
chunweii opened this issue Oct 17, 2022 · 5 comments
Labels
category.Bug difficulty.Easy suitable for beginners good first issue p.Medium to be done when there are no other higher priority issues

Comments

@chunweii
Copy link
Contributor

Describe the bug
During the tester response phase, the text of the team/tester response in the issue comment on github will contain a warning message at the start. An example:

[IMPORTANT!: Please do not edit or reply to this comment using the GitHub UI. You can respond to it using CATcher during the next phase of the PE]

# Team's Response
Duplicate marking test

* List
  * List Item 1
  * List Item 2

# Items for the Tester to Verify

## :question: Issue severity

Team chose [`severity.Low`]
Originally [`severity.VeryLow`]

- [ ] I disagree

**Reason for disagreement:** [replace this with your reason]

-------------------

After the tester submits the response, the warning message will disappear. For example:

# Team's Response
Duplicate marking test

* List
  * List Item 1
  * List Item 2
# Items for the Tester to Verify
## :question: Issue severity

Team chose [`severity.Low`]
Originally [`severity.VeryLow`]

- [x] I disagree

**Reason for disagreement:** hello

-------------------

The warning should remain in the GitHub comments to warn testers against editing directly their response on GitHub directly.

@chunweii chunweii added category.Bug p.Low low value, can be delayed indefinitely labels Oct 17, 2022
@damithc damithc added p.Medium to be done when there are no other higher priority issues and removed p.Low low value, can be delayed indefinitely labels Oct 17, 2022
@Echomo-Xinyu Echomo-Xinyu self-assigned this Jan 15, 2023
@Echomo-Xinyu
Copy link

[IMPORTANT!: Please do not edit or reply to this comment using the GitHub UI. You can respond to it using CATcher during the next phase of the PE]

I am unsure about the second part of the message here: the tester response phase should be the last phase in PE that students are involved in, so I don't think it is possible for testers to respond during the next phase?

It seems to me that the second sentence of the message here is either outdated (inherited from the dev-response phase) or should be removed.

@damithc
Copy link
Contributor

damithc commented Jan 16, 2023

Good point. These issues are created by a script after the dev response phase has ended but before the tester response phase officially begun (there is a gap in between). Hence, 'next' refers to the tester response phase.

@Echomo-Xinyu
Copy link

Then in this case, will it be better to keep only the first part of the warning? ie after editing/submitting, instead of having the first line completely disappear (which is its current behaviour), it will be replaced with its first part.

Before editing/submitting:

[IMPORTANT!: Please do not edit or reply to this comment using the GitHub UI. You can respond to it using CATcher during the next phase of the PE]

# Team's Response
Duplicate marking test

* List
  * List Item 1
  * List Item 2

# Items for the Tester to Verify

## :question: Issue severity

Team chose [`severity.Low`]
Originally [`severity.VeryLow`]

- [ ] I disagree

**Reason for disagreement:** [replace this with your reason]

-------------------

after editing/submitting:

[IMPORTANT!: Please do not edit or reply to this comment using the GitHub UI.]

# Team's Response
Duplicate marking test

* List
  * List Item 1
  * List Item 2

# Items for the Tester to Verify

## :question: Issue severity

Team chose [`severity.Low`]
Originally [`severity.VeryLow`]

- [ ] I disagree

**Reason for disagreement:** [replace this with your reason]

-------------------

@damithc
Copy link
Contributor

damithc commented Jan 16, 2023

We need to keep this string exactly as it is, because the same string is referenced by the issue processing script that runs between phases. If the string needs rephrasing, it needs to be done on both CATcher and script side at the same time -- but that can be a separate issue as the bug reported here is about the disappearance of the string only.

@Echomo-Xinyu
Copy link

Noted! Then I shall stick to the original problem of not removing the warning. Thanks for the clarification!

@chunweii chunweii pinned this issue Mar 26, 2024
@cheehongw cheehongw unpinned this issue Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category.Bug difficulty.Easy suitable for beginners good first issue p.Medium to be done when there are no other higher priority issues
Projects
None yet
Development

No branches or pull requests

4 participants