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

Bugfix to update stream marker/color on editing stream message #1181

Merged
merged 2 commits into from
Mar 30, 2022

Conversation

neiljp
Copy link
Collaborator

@neiljp neiljp commented Mar 30, 2022

What does this PR do?

If editing a message in a private stream (or soon, a web-public stream), the stream marker was not set correctly.

One commit adjusts this to do so correctly using the new helper method, while the other removes the duplicate code originally used prior to the helper method.

Tested?

  • Manually
  • Existing tests (adapted, if necessary)
  • New tests added (for any new behavior)
  • Passed linting & tests (each commit)

Notes & Questions

We could add tests for the stream marker/color setting in various edit modes - we have none currently. The marker/color behavior is tested separately using a test for the common helper method.

This could previously be triggered if editing a stream message in a
private stream.
We re-calculate the color and marker for a stream consistently in the
new-stream-message or edit-stream-message setup methods, making this
older code unnecessary.

In principle this might be incorporated in the common method, but it is
clearer in the individual methods due to differing usage.
@neiljp neiljp added bug Something isn't working PR soon to be merged PR has been reviewed & is ready to be merged area: message editing labels Mar 30, 2022
@zulipbot zulipbot added the size: S [Automatic label added by zulipbot] label Mar 30, 2022
@neiljp neiljp added this to the Next Release milestone Mar 30, 2022
@neiljp neiljp merged commit bb84d21 into zulip:main Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: message editing bug Something isn't working PR soon to be merged PR has been reviewed & is ready to be merged size: S [Automatic label added by zulipbot]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants