Skip to content

Remove contrib status and show meaningful contrib only#63

Closed
JanStevcik wants to merge 9 commits intowikimedia:masterfrom
JanStevcik:master
Closed

Remove contrib status and show meaningful contrib only#63
JanStevcik wants to merge 9 commits intowikimedia:masterfrom
JanStevcik:master

Conversation

@JanStevcik
Copy link
Copy Markdown

Hi, I think I've solved issue #60 and #61.
Hope it's solved good.


COMMIT_STATUS = ['merged', 'open', 'abandoned', 'declined', 'resolved', 'stalled', 'invalid',
'closed', 'pending', 'reviewed', 'p-open', 'g-open']
# COMMIT_STATUS = ['merged', 'open', 'abandoned', 'declined', 'resolved', 'stalled', 'invalid',
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There is no need to comment the present one out!! You can replace it.

@rammanoj
Copy link
Copy Markdown
Collaborator

rammanoj commented Nov 1, 2019

@JanStevcik Thanks for the contribution.

You can not solve #61 There is a tag named "Google Code-In" added to it. It means that only the people who are participating in code-In can solve the issues. At present they are reserved for them. To know more about Google CodeIn you can visit https://codein.withgoogle.com/

Coming to #60 It aims at removing only the status filters. But I guess you removed the entire code of the filters. This does not solve an issue but it further creates more issues.

Let me know if you have any queries.

@JanStevcik
Copy link
Copy Markdown
Author

Hi, I create new PR, hope now is filters removed fine.

@JanStevcik
Copy link
Copy Markdown
Author

@rammanoj
Hi, hope status filters are remove fine now.

In last commit I've removed WMContraband. Hope now it's good.

@rammanoj
Copy link
Copy Markdown
Collaborator

rammanoj commented Nov 2, 2019

@JanStevcik Thanks for your interest :)

I guess you made few minor mistakes. It appears like you have also added the semantic/ directory to the commit. semantic/ is just a dependency. You should not add it in the commits.

Can you please resolve it ?

@JanStevcik
Copy link
Copy Markdown
Author

Hi, I remove it. :)

@rammanoj
Copy link
Copy Markdown
Collaborator

rammanoj commented Nov 5, 2019

Cool @JanStevcik. I am sorry for the delay, this PR is few more steps away from getting merged :)

These are the changes that need an update:

  1. You added a new file named db.sqlite3. These store all the local data (i.e local sqlite3 database file). So, please do not add it to the PR.
  2. The file backend/Contraband/query/management/__pycache__/__init__.cpython-36.pyc is the compiled version of management query __init__.py file. Do not add it to the PR.
  3. Same with the file semantic.json. Please remove it from the PR.
  4. Coming to the actual code UserUpdateTimeStamp class is used to update the timestamp, you are only removing the status filter of the user. The class UserUpdateTimeStamp is used to update the timestamp filter. So, please don't remove it's code.

@JanStevcik
Copy link
Copy Markdown
Author

I'm sorry, I think UserUpdateTimeStamp is part of filters that should be removed.
Thanks very much for your time and patience! ☺
I hope, now it's good.

@rammanoj
Copy link
Copy Markdown
Collaborator

rammanoj commented Nov 9, 2019

Thanks for your contribution @JanStevcik It looks good to me now. I will check it once in my localhost and get back..

@JanStevcik
Copy link
Copy Markdown
Author

JanStevcik commented Nov 15, 2019

Hi @rammanoj, I just remind this issue, is everything ok, or is there some problems? Thanks.

@srish
Copy link
Copy Markdown
Member

srish commented Nov 21, 2019

@JanStevcik Thanks for your patience! I just tested your code locally. It seems like the filters are completely gone now. There used to be "View filters" button next to the edit button that is missing now:
Screen Shot 2019-11-20 at 4 15 48 PM

Ideally, the date and time range remains and filter section looks like this:
Screen Shot 2019-11-20 at 4 18 14 PM

@rammanoj
Copy link
Copy Markdown
Collaborator

rammanoj commented Nov 22, 2019

I am sorry @JanStevcik for keeping you wait so long. Most of time was occupied with semester exams and final year project. Coming to the changes you made, I even tried it now on my localhost, these are not the exact intended ones, there need to be some more changes to be made...

Three things need to be done:

  1. I guess you only need to remove:
 <Grid.Row>
                        <Grid.Column computer={16} tablet={16} mobile={16}>
                          <Header>Contribution Status</Header>
                          <Dropdown
                            style={{ marginTop: 10 }}
                            fluid
                            search
                            multiple
                            selection
                            options={format_status(
                              gerrit_status.concat(phab_status, 'open')
                            )}
                            value={uf.status}
                            onChange={(e, obj) => {
                              let value = obj.value;
                              let filters = Object.assign({}, uf);
                              filters.status = value;
                              this.setState({
                                update_filters: filters,
                              });
                            }}
                            placeholder="Status of Commit"
                            closeOnChange={true}
                          />
                        </Grid.Column>
                      </Grid.Row>

the above code to prevent Status of commit from being displayed.

  1. On removing the above code, the function format_status() becomes useless.

  2. There is also no need of status in filters and update_filters state variables. So, you need to remove all the occurrences of it in the code. Few of them are:

filters.status = data.filters.status.split(','); //Line 159 in your master branch
filters.status = filters.status.split(','); // Line 289 in your master branch

etc. There are few more deletions need to be done (similar to the above statements).

Coming to the backend:

You have correctly removed the class, but you also need to remove all the occurrences of the class. You can take a look here: https://github.com/wikimedia/WikiContrib/blob/master/backend/Contraband/query/views.py#L375

The class is being used over there. In the similar manner you need to find all the occurrences of the class and remove them :)

@JanStevcik
Copy link
Copy Markdown
Author

JanStevcik commented Dec 2, 2019

Hi, i'm sorry for I didn't answer too long, but I had some work and study duties.

I trying to fix problems that you describe, but I can not connect to server. I always get response code 500. I was generate new API_TOKEN but I have still the same problem. I was try run master branch from main project, but there is the same problem, response code 500.

So I was try POST data via Postman and it does not work. I think, issue is on server side. So I can't check how is filters in frontend looks now.

There is postman output:
image

Thanx!

@rammanoj
Copy link
Copy Markdown
Collaborator

rammanoj commented Dec 4, 2019

Thanks for reporting this @JanStevcik . I updated the server recently. Updating the server takes a lot of effort. There might be some minor error while I was updating. I will check it out and report you by today....

@rammanoj
Copy link
Copy Markdown
Collaborator

rammanoj commented Dec 4, 2019

@JanStevcik the exact reason along with the PR for the server breaking is specified at #81. Once again thanks for reporting the error. I haven't observed it :)

@rammanoj rammanoj force-pushed the master branch 4 times, most recently from c7172f1 to 50d7881 Compare December 5, 2019 06:48
@rammanoj rammanoj force-pushed the master branch 29 times, most recently from 935d24d to bfd80fe Compare December 8, 2019 06:53
@srish srish closed this Mar 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants