D61-IA / stellar-gnosis

Gnosis paper management and collaboration tool
Apache License 2.0
0 stars 1 forks source link

Bug/pagination fix #88

Closed Zhenghao-Zhao closed 4 years ago

Zhenghao-Zhao commented 4 years ago

Features:

Note:

Zhenghao-Zhao commented 4 years ago

All tests passed. I have no more updates. Thanks!

PantelisElinas commented 4 years ago

Hi @Zhenghao-Zhao

can you have a look at the conflicts with the develop branch so that I can review this?

Thank you!

Zhenghao-Zhao commented 4 years ago

Conflict resolved.

PantelisElinas commented 4 years ago

Hi @Zhenghao-Zhao

am I correct that this pull request includes all the code updates in #86 as well? Because it looks like it. Did you create a branch from the #86 branch for implementing this fix?

Zhenghao-Zhao commented 4 years ago

Yes. This was done after our meeting last Thursday, where you suggested the fix for report error (add reporter name to report, and Django tests). I thought the fix was pretty straight forward, therefore I just created the branch off report error after I fixed it so as to avoid conflicts with that branch.

PantelisElinas commented 4 years ago

Yes, but if I approve this pull request and merge with develop, then I am also merging the code from #86 before I had a chance to review it separately. This fix should have been a branch from develop. Alternatively, if it was something small, then you could have fixed it in #86 instead of making a separate pull request. That said, the latter is not good practice as you should not be mixing tickets with your pull requests.

Anyway, keep this in mind for the future, I will just have a look at #86 first and then this one.

PantelisElinas commented 4 years ago

Hi @Zhenghao-Zhao

I merged #86 and now this one has many conflicting files. Can you please fix when you have time?

Thank you!

Zhenghao-Zhao commented 4 years ago

Still under testing... will let you know when it is ready for review. Thanks.

Zhenghao-Zhao commented 4 years ago

All tests passed. I also added pagination to other templates. Let me know if I missed any.

Note if you see a message read "session not created: This version of ChromeDriver only supports Chrome version 78 error with ChromeDriver Chrome using Selenium", that means you have to download the Chrome driver that works for your version. I think the latest one always works for older ones but not confirmed.