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

Script should follow user talk page redirects #90

Closed
bardiharborow opened this issue Feb 3, 2018 · 17 comments · Fixed by #291 or #322
Closed

Script should follow user talk page redirects #90

bardiharborow opened this issue Feb 3, 2018 · 17 comments · Fixed by #291 or #322
Labels
C-bug Category: bug E-good-first-issue Experience needed: zero (good first issue) high-priority Requested by different people on different occasions

Comments

@bardiharborow
Copy link
Contributor

The script should not have made this edit, but instead should have followed the talk page redirect and left a message at the redirect target.

@enterprisey enterprisey added the C-enhancement Category: enhancement label Jan 16, 2021
@enterprisey enterprisey changed the title Script should follow talk page redirects Script should follow user talk page redirects Jan 18, 2021
@enterprisey enterprisey added C-bug Category: bug and removed C-enhancement Category: enhancement labels Jan 18, 2021
@enterprisey
Copy link
Contributor

To reproduce: comment on a draft, with the "Notify user" option enabled, submitted by a user whose talk page is a redirect. Currently, the script will send a notice to the redirect, not the target of the redirect.

I'm pretty confident that all we need to do is add the parameter redirect: "true" to the API call that we use to notify users. The function that notifies users is notifyUsers in core.js, which uses editPage in core.js. While we could solve this by adding redirect: "true" to the request in editPage, that'll probably mess up other things. So, we should add a way for the callers of editPage to ask it to set redirect: "true", perhaps by handling options.followRedirect (to use a more obvious name than just "redirect").

@enterprisey enterprisey added the E-good-first-issue Experience needed: zero (good first issue) label Jan 18, 2021
@primefac
Copy link

As a note, Twinkle has the same issue so if this one gets solved it might be good to let Amory know so that it can be fixed in both locations.

@puneetdwivedi
Copy link

i want to work on this. how i can start

@enterprisey
Copy link
Contributor

@puneetdwivedi Sorry! Not sure how I missed this. Here are instructions on getting set up; if you run into any trouble, post here or on the other help venues.

@enterprisey
Copy link
Contributor

@puneetdwivedi How is this issue going? Please let me know if you've run into any problems.

@Ankur21980
Copy link

Hello! @enterprisey, I would like to work on this issue. Is it open yet?

@enterprisey
Copy link
Contributor

@Ankur21980 hi! I would like to not assign this issue to someone else just yet - if you really want to work on this issue, please wait until the person currently assigned has not responded for another week. Alternatively, please ping me in a new topic in our Zulip channel - there are more issues that do not have the good-first-issue tag that might be easy to work on, depending on your level of experience.

@puneetdwivedi - are you still available to work on this issue? Unfortunately easy issues on this repository are not that easy to find, so please let me know whether or not you plan to continue working on this issue. If you do not make a comment on this issue with your status by a week from now (the 27th of March) I will assume you are not available, and will mark this issue as open for anyone to take.

@Prernajha2609
Copy link

hello @enterprisey, I would like to contribute to this project, I will need a little guidance to start.

@enterprisey
Copy link
Contributor

@Prernajha2609
Copy link

how should I start after setting up the environment?

@NovemLinguae
Copy link
Member

I imagine you'd want to pick an easy issue from the list of issues, write a patch, test it using https://test.wikipedia.org/, then submit a pull request.

@Prernajha2609
Copy link

Prernajha2609 commented Jan 16, 2022 via email

@NovemLinguae
Copy link
Member

@primefac
Copy link

Is #99 a duplicate of this issue?

sohomdatta1 added a commit to sohomdatta1/afch-rewrite that referenced this issue Aug 29, 2023
sohomdatta1 added a commit to sohomdatta1/afch-rewrite that referenced this issue Aug 30, 2023
sohomdatta1 added a commit to sohomdatta1/afch-rewrite that referenced this issue Sep 17, 2023
@NovemLinguae
Copy link
Member

@NovemLinguae NovemLinguae added the high-priority Requested by different people on different occasions label Jan 27, 2024
NovemLinguae added a commit to NovemLinguae/afc-helper that referenced this issue Apr 10, 2024
Fixes wikimedia-gadgets#90

Advantages
- Simpler than patch wikimedia-gadgets#291
- The original intent of the function notifyUser was to follow redirects, indicated by the comment "Follows redirects and appends a message to the bottom of the user's talk page." from 2014. So this fixes that bug.

Disadvantages
- Will only follow 1 redirect, not 2+ redirects.
- The "Saved [[User talk:OldUsername]] (diff)" message shown to the AFC reviewer will be slightly incorrect. The diff will be correct, but OldUsername should ideally be NewUsername.
NovemLinguae pushed a commit to sohomdatta1/afch-rewrite that referenced this issue Apr 12, 2024
siddharthvp pushed a commit that referenced this issue Apr 13, 2024
Fixes #90

Advantages
- Simpler than patch #291
- The original intent of the function notifyUser was to follow redirects, indicated by the comment "Follows redirects and appends a message to the bottom of the user's talk page." from 2014. So this fixes that bug.

Disadvantages
- Will only follow 1 redirect, not 2+ redirects.
- The "Saved [[User talk:OldUsername]] (diff)" message shown to the AFC reviewer will be slightly incorrect. The diff will be correct, but OldUsername should ideally be NewUsername.
NovemLinguae added a commit that referenced this issue Apr 15, 2024
* Recursively identify renamed username of submitter

Fixes #90

* incorporate most code review suggestions

* add comment

---------

Co-authored-by: NovemLinguae <[email protected]>
@NovemLinguae
Copy link
Member

These two patches did not fix the issue apparently.

Steps to reproduce:

  • submit -> someone else -> NovemBot
  • decline
  • reason: v
  • comment: test
  • tick notify user
  • tick invite to teahouse
  • decline

What happens?

What should happen instead?

  • comment should be on User talk:Novem Linguae, not User talk:NovemBot
  • the comment is invisible due to the redirect above it

@NovemLinguae NovemLinguae reopened this Apr 23, 2024
@NovemLinguae
Copy link
Member

Will spin the bug out into its own ticket. #349

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug E-good-first-issue Experience needed: zero (good first issue) high-priority Requested by different people on different occasions
Projects
None yet
7 participants