feature/documents#25
Conversation
jzitnik-dev
left a comment
There was a problem hiding this comment.
You also can't just ignore the .. page but in the DocumentsPage class add something like parentPath that would include the path of the parent folder, so then in the UI you can navigate back normally as expected without needing to explicitly track your path history.
|
Thanks for review, will fix tommorow. |
|
Fixed it. |
|
Btw please write better commit names. Once you are making a PR, they aren't just your no-one-cares commit messages, but they are merged into the project. |
jzitnik-dev
left a comment
There was a problem hiding this comment.
I don't like how the elements in the page are getting parsed twice using the select function for the parent folder, due to performance (select has to go thru all the elements in the page to see if it matches the css selector), but it makes the code look cleaner with the .mapNotNull { parseDocument(it) }.
The thing I personally hate is assuming the first folder will be the parent dir. Obv it would make sense but I think way better approach would be to go through all the files and folders just once, and instead of skipping the ".." (as you did before) just setting the parentPath in it yk. So you eliminate going through all the documents and folders twice and you also eliminate the possibility if the parent folder isn't the first folder in the HTML tree.
|
Ready to merge? |
|
I haven't reviewed the code yet, so no |
|
Is Jakub a code reviewer or not? If no, it would make sense. |
|
No, but maybe he could be. |
|
My review was just my personal opinion xd. Final word is on @tomhula. |
Add the ability to fetch and parse the school documents, subfolders, and files. Example usage:
runBlocking {
val client = JecnaClient()
client.login(username, password)
val visited = mutableSetOf()