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

Zulip topic is only renamed for messages after MCP issue rename occurs #1228

Closed
camelid opened this issue Mar 6, 2021 · 5 comments · Fixed by #1590
Closed

Zulip topic is only renamed for messages after MCP issue rename occurs #1228

camelid opened this issue Mar 6, 2021 · 5 comments · Fixed by #1590
Labels
bug Something isn't working

Comments

@camelid
Copy link
Member

camelid commented Mar 6, 2021

It happened here. triagebot renamed the topic of the "renaming this Zulip topic" message, which effectively splits the topic in two, when it should have renamed the topic of the original "new proposal" message.

The code that is responsible for renaming the topic is here:

let zulip_update_req = crate::zulip::UpdateMessageApiRequest {
message_id: zulip_send_res.message_id,
topic: Some(&new_topic),
propagate_mode: None,
content: None,
};

Another bug is that renaming the Zulip topic breaks the link in the MCP issue—which is what #711 was originally trying to fix (see #701, the short summary is that someone manually renamed the Zulip topic, which broke the link in the MCP issue).

Fixing these issues is probably going to be difficult. I'm not sure what the right solution is.

@camelid
Copy link
Member Author

camelid commented Mar 6, 2021

See also #706, which was another way to approach #701.

@camelid
Copy link
Member Author

camelid commented Mar 6, 2021

@rustbot label: bug

@rustbot rustbot added the bug Something isn't working label Mar 6, 2021
@ghost
Copy link

ghost commented Mar 13, 2022

I was recently bit by this and ended up with 3 topics for my MCP. Thankfully the discussion didn't fork and stayed on the originally named topic.

I think this can be fixed by just using the change_all propagate_mode when editing the topic. See also Add API endpoint for renaming topics.

I also just noticed that change_all was already discussed in #701, but based on this comment it looks like the update message API didn't exist at the time and was only added later in #711. I'm not sure if there was some issue originally that prevented change_all from being used in #711 from the start.

I'm going to post a PR as this is just a simple one line fix (in theory...), however I don't have any kind of triagebot dev instance running to test this out myself.

@ghost
Copy link

ghost commented Mar 13, 2022

Oh, I've posted my PR, but now I think I may have misunderstood this bug. Is the issue not that the "Zulip topic is only renamed for messages after MCP issue rename occurs", but instead that doing so breaks the links in the MCP?

Mark-Simulacrum added a commit that referenced this issue Mar 13, 2022
Rename the entire zulip topic when an MCP issue is renamed. Fixes #1228.
@camelid
Copy link
Member Author

camelid commented Mar 13, 2022

I think change_all didn't exist before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants