Skip to content
This repository was archived by the owner on Jan 23, 2025. It is now read-only.

Improve challenge visibility control: getChallenge and getRegistrants #504

Merged
merged 7 commits into from
Jun 20, 2017

Conversation

TheOsch
Copy link
Contributor

@TheOsch TheOsch commented Jun 20, 2017

Improve challenge visibility control: getChallenge and getRegistrants

I removed one field from the check_is_related_with_challenge query. Besides getChallenge there are two other functions in actions/challenges.js that use this query: getSubmissions and getPhases. These two functions use this query only to check the eligibility. I decided to take the liberty of fixing them too. So now six more actions work according to the new logic: getChallenge, getSoftwareChallenge, getStudioChallenge (all these three actions in fact consist of a simple call to the same function - getChallenge), getRegistrants, getSubmissions and getRoles.
Postman tests are provided for getChallenge, getRegistrants, getSubmissions and getRoles actions.

@skyhit skyhit merged commit df292bf into topcoder-archive:dev Jun 20, 2017
@skyhit
Copy link
Contributor

skyhit commented Jun 20, 2017

@TheOsch thanks, looks good, after fixing this, what endpoints are left for challenges?

@TheOsch
Copy link
Contributor Author

TheOsch commented Jun 20, 2017

I saw you have changed a number of queries by adding a group id check there. I think now first it would be good to update all places where these queries are used and remove toese checks, wouldn't it? I think I can make it in a day.

@TheOsch
Copy link
Contributor Author

TheOsch commented Jun 20, 2017

It may appear that this operation will empty our list of endpoints to change leaving only those that were included there by mistake.

skyhit pushed a commit that referenced this pull request Jun 20, 2017
* Improve challenge visibility control (#501)

* IMPROVE CHALLENGE VISIBILITY CONTROL
(https://www.topcoder.com/challenge-details/30057891/?type=develop)

Verification guide: docs/Verification_Guide-Improve Challenge Visibility Control.doc

* Restoring an accidentially modified file

* Fixed the case with a challenge that doesn't have eligibility

* Shared the eligibility verification with challengeRegistration.
The eligibility check routine is now in challengeHelper and can be added anywhere by a couple of simple lines of code.

* improve the query

* update query for groups (#502)

* Update queries (#503)

improve logging for v3 api call

* should use externalToken field name

* update queries for group checking

* Improve challenge visibility control: getChallenge and getRegistrants (#504)

* IMPROVE CHALLENGE VISIBILITY CONTROL
(https://www.topcoder.com/challenge-details/30057891/?type=develop)

Verification guide: docs/Verification_Guide-Improve Challenge Visibility Control.doc

* Restoring an accidentially modified file

* Fixed the case with a challenge that doesn't have eligibility

* Shared the eligibility verification with challengeRegistration.
The eligibility check routine is now in challengeHelper and can be added anywhere by a couple of simple lines of code.

* Improve challenge visibility control: getChallenge and getRegistrants

* revert commit
@TheOsch
Copy link
Contributor Author

TheOsch commented Jun 20, 2017

By the way, is there an endpoint in V3 that returns a list of groups the given user belongs to? It would be very useful.

@ajefts
Copy link
Contributor

ajefts commented Jun 20, 2017

@TheOsch
Copy link
Contributor Author

TheOsch commented Jun 20, 2017

@ajefts I couldn't get a valid auth token to call the service. Can you please show me a sample output of this call?

@ajefts
Copy link
Contributor

ajefts commented Jun 20, 2017

Sure.

{
    "id": "25ab1528:15cc0ff4067:1c6a",
    "result": {
        "success": true,
        "status": 200,
        "metadata": null,
        "content": [
            {
                "id": "20000000",
                "modifiedBy": "8547899",
                "modifiedAt": "2017-05-25T09:31:10.000Z",
                "createdBy": "8547899",
                "createdAt": "2017-05-25T09:31:10.000Z",
                "name": "Group Name",
                "description": "Overall XYZ Group"
            }
        ]
    },
    "version": "v3"
}

@TheOsch
Copy link
Contributor Author

TheOsch commented Jun 20, 2017

@ajefts Thank you

@skyhit skyhit mentioned this pull request Jun 21, 2017
skyhit added a commit that referenced this pull request Jun 21, 2017
* Improve challenge visibility control (#501)

* IMPROVE CHALLENGE VISIBILITY CONTROL
(https://www.topcoder.com/challenge-details/30057891/?type=develop)

Verification guide: docs/Verification_Guide-Improve Challenge Visibility Control.doc

* Restoring an accidentially modified file

* Fixed the case with a challenge that doesn't have eligibility

* Shared the eligibility verification with challengeRegistration.
The eligibility check routine is now in challengeHelper and can be added anywhere by a couple of simple lines of code.

* improve the query

* update query for groups (#502)

* Update queries (#503)

improve logging for v3 api call

* should use externalToken field name

* update queries for group checking

* Improve challenge visibility control: getChallenge and getRegistrants (#504)

* IMPROVE CHALLENGE VISIBILITY CONTROL
(https://www.topcoder.com/challenge-details/30057891/?type=develop)

Verification guide: docs/Verification_Guide-Improve Challenge Visibility Control.doc

* Restoring an accidentially modified file

* Fixed the case with a challenge that doesn't have eligibility

* Shared the eligibility verification with challengeRegistration.
The eligibility check routine is now in challengeHelper and can be added anywhere by a couple of simple lines of code.

* Improve challenge visibility control: getChallenge and getRegistrants

* revert commit
@TheOsch
Copy link
Contributor Author

TheOsch commented Jun 21, 2017

@skyhit
Bad news: I was too presumptuous when told that I can complete everything in one day. I need one more day, will do a PR tomorrow
Good news: I checked the entire list and explored the source code of every 'suspicious' endpoiont. That PR willl be really the last one in this series, it will cover everything.

@skyhit
Copy link
Contributor

skyhit commented Jun 21, 2017

@TheOsch ok, that is fine.

@TheOsch
Copy link
Contributor Author

TheOsch commented Jun 26, 2017

Made a PR on Friday bit forgot to notify you. Did you notice it?

@skyhit
Copy link
Contributor

skyhit commented Jun 26, 2017

@TheOsch you mean #508?

@TheOsch
Copy link
Contributor Author

TheOsch commented Jun 26, 2017

Yes

@skyhit
Copy link
Contributor

skyhit commented Jun 26, 2017

@TheOsch ok, I will review that

@TheOsch
Copy link
Contributor Author

TheOsch commented Jun 26, 2017

I had to tell you 3 days ago but I thought you'll be automatically notified about it. Sorry.

@skyhit
Copy link
Contributor

skyhit commented Jun 26, 2017

@TheOsch some of the changes seems already applied, can you merge latest dev? so I can see the difference clearly?

@TheOsch
Copy link
Contributor Author

TheOsch commented Jun 26, 2017

The latest commit to dev was earlier than the PR, and I merged just before pushing. Right now the attempt to merge results in 'Already up-to-date.' message, and status reports there's nothing to commit.

@TheOsch
Copy link
Contributor Author

TheOsch commented Jun 26, 2017

Which changes are suspicious?

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

Successfully merging this pull request may close these issues.

3 participants