Fixes #2507 - Display needsinfo requests in user activity page #2544
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Actually... maybe @karlcow can review the python bits before he goes on PTO. |
|
(and then I'll ping someone else for review on functional tests and JS) |
karlcow
requested changes
Jul 6, 2018
There was a problem hiding this comment.
OK let's see http://localhost:5000/activity/karlcow
This is working well. 👍
- Remove the print statement
- and I'll leave it up to you for the format string which is basically not part of this pull request but the previous coding.
webcompat/api/endpoints.py
Outdated
| path = 'repos/{path}'.format(path=ISSUES_PATH) | ||
| print(path, params) |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
webcompat/api/endpoints.py
Outdated
| if parameter == 'needsinfo': | ||
| params['labels'] = 'status-needsinfo-{0}'.format(username) | ||
| else: | ||
| params['{0}'.format(parameter)] = '{0}'.format(username) |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
Travis tests have failedHey @miketaylr, 1st Buildnosetestssleep 5npm run test:js -- --reporters="runner" --firefoxBinary=`which firefox` |
|
OK, python nits addressed. r? @magsout for non-python review parts. |
|
@miketaylr it's on my todo list. Lot of working travel ;) |
|
@miketaylr goog job. Just few question about UX. There is 3 section in this page, so 3 paginations. But whatever the pagination you're clicking every section is updated. It looks weird, nah, what do you think? |
|
following my previous question, I think we should hide pagination if there is not result:
I think is not depends of it view, it seems general, isn't it ? |
Yeah, that feels like a bug.
Yeah, good idea! |
karlcow
approved these changes
Jul 26, 2018
In theory, the current behavior is to add a "is-disabled" class. This seems to work on the issue list page, but no here. So, some bug somewhere. If I can find it, let's fix that then open a follow up bug to swap "is-disabled" with "is-hidden" (or something like that). |
Hey, it turns out that's our default behavior. :p Been a long time since I've looked at this code. The bug was that all the shared stuff was trying to paginate on the "my issues" stuff. I've made sure it operates on the correct instance instead. r? @magsout |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes issue #2507
Proposed PR background
Just adding another section to the activity page, showing needsinfo requests.
Need to add some functional tests before I request review.