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

Add a Staging Ground message indicator to reports on Staging Ground posts #7827

Closed
wants to merge 2 commits into from
Closed

Add a Staging Ground message indicator to reports on Staging Ground posts #7827

wants to merge 2 commits into from

Conversation

Papershine
Copy link
Member

As mentioned in #7649, it would be helpful to indicate that if a post might be hidden due to being in the Staging Ground. This pull requests implements one of the suggestions of displaying that information in the message report. Sample:

[ SmokeDetector | MS ] No whitespace in title (71): SomeQuestion‭ (Staging Ground) by some-user on stackoverflow.com

Note: This change might potentially break certain userscripts on those reports?

@tripleee
Copy link
Member

tripleee commented Jun 4, 2023

An additional complication might be how it affects the logic for forcing the final message to be within the maximum length allowed by the chat API. I'm not in a place where I can investigate that, but I'm noting it for when we can do a proper review.

Thanks for the contribution!

Copy link
Contributor

@makyen makyen left a comment

Choose a reason for hiding this comment

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

This "request changes" review is really "needs verification, with possible needs changes". With Staging Ground currently shut down and empty, I'm unable to verify that post.post_url will have a URL which contains the text "staging-ground" (note: "/staging-ground/" would be a better test). The post.post_url is derived from the link value returned from the SE API for the question. I suspect that what will be supplied by the SE API is the canonical /questions/<post ID>/... URL, rather than the staging ground URL.

SD was changed to parse /staging-ground/<post ID> URLs, but that seems to have been as a result of my realizing that SD needs to do that in order to accommodate user's providing such URLs in chat commands, rather than as the link value returned by the SE API.

If you verified that the link value provided by the SE API has "staging-ground" in it prior to submitting this PR, then that's sufficient for this concern.

@Papershine
Copy link
Member Author

I recall verifying with the SE API that they also return a link with staging-ground in it somewhere, but my memory is hazy about the exact format. Will reconfirm when (or if) Staging Ground is online again.

@codygray
Copy link
Member

codygray commented Sep 7, 2024

I wanted to re-open this, but I cannot do so because "The repository that submitted this pull request has been deleted." Instead, I posted a new issue.

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

Successfully merging this pull request may close these issues.

4 participants