Skip to content
This repository has been archived by the owner on Jan 10, 2024. It is now read-only.

Network Layer and Model Refactor #167

Merged
merged 96 commits into from
Nov 2, 2017
Merged

Conversation

nerdsupremacist
Copy link
Member

@nerdsupremacist nerdsupremacist commented Oct 19, 2017

This redoes the entire networking layer and removes SwiftyJSON and Alamofire from our dependencies.

It makes our networking a bit more type-safe and relies less on casting.

And it introduces caching

Fixes #72
Fixes #109
Starts progress in #43 & #165
Fixes #156
Fixes #90

While I was at it I also fixed: #121 with the same workaround as android

nerdsupremacist and others added 30 commits April 28, 2017 21:58
…update

# Conflicts:
#	Pods/Pods.xcodeproj/project.pbxproj
#	Pods/Target Support Files/Pods-Campus/Pods-Campus.debug.xcconfig
#	Pods/Target Support Files/Pods-Campus/Pods-Campus.release.xcconfig
#	TUM Campus App.xcodeproj/project.pbxproj
# Conflicts:
#	Pods/Pods.xcodeproj/project.pbxproj
#	Pods/Target Support Files/Pods-Campus/Pods-Campus-resources.sh
#	Pods/Target Support Files/Pods-Campus/Pods-Campus.debug.xcconfig
#	Pods/Target Support Files/Pods-Campus/Pods-Campus.release.xcconfig
#	TUM Campus App/TumDataManager.swift
# Conflicts:
#	Pods/Pods.xcodeproj/project.pbxproj
# Conflicts:
#	Podfile.lock
#	TUM Campus App.xcodeproj/project.pbxproj
#	TUM Campus App/CalendarManager.swift
#	TUM Campus App/Extensions.swift
@nerdsupremacist
Copy link
Member Author

@TG908 Can you please be more specific?

@tgymnich
Copy link
Member

I’m getting no Library Card and no success message on login.

@tgymnich
Copy link
Member

@mathiasquintero the card doesn't show up and when trying to log in theres no success alert

@tgymnich
Copy link
Member

I think i found the bug: // TODO: Access BookRentalsManager

@tgymnich
Copy link
Member

@mathiasquintero Do you know if the search Manager can also search in categories like, people, rooms and lectures?

@nerdsupremacist
Copy link
Member Author

@TG908 Ok. I'm fixing it

@nerdsupremacist
Copy link
Member Author

nerdsupremacist commented Oct 26, 2017

@TG908 I don't unserstand your question. We don't have a single search manager. We have many that are concatenated whenever you perform a search.

These search for People, Rooms and Lectures. But we can always extend them. Just look for TumDataManager.searchManagers

We could make it search for books, or search for movies or search for Tum Sexy entries

@nerdsupremacist
Copy link
Member Author

@mammuth @TG908 any other bugs or concerns, because otherwise, I think I'm done with this one...
And if we see any more bugs later we can also fix them

keychainWrapper.writeToKeychain()
UserDefaults.standard.synchronize()
}
// TODO: Refactor use of keychain wrapper
Copy link
Collaborator

Choose a reason for hiding this comment

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

INFO Complete the task associated to this "TODO" comment. rule

UserDefaults.standard.synchronize()
}
// TODO: Refactor use of keychain wrapper
// TODO: Move this to the manager. This is really the managers job.
Copy link
Collaborator

Choose a reason for hiding this comment

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

INFO Complete the task associated to this "TODO" comment. rule

@tgymnich
Copy link
Member

I think we should create a new branch and start integration of all the changes.

@nerdsupremacist
Copy link
Member Author

nerdsupremacist commented Oct 26, 2017

@TG908 I'm not sure what you mean by integration. This is all integrated. What we're missing is the new Cards and UIs we have planned

@tgymnich
Copy link
Member

tgymnich commented Oct 26, 2017

@TG908 I'm not sure what you mean by integration. This is all integrated. What we're missing is the new Cards and UIs we have planned

Did you change the search controller ? I want to add a segment control to the search bar. Also for the new Cards/UI we need a fully integrated branch

@nerdsupremacist
Copy link
Member Author

@TG908 I haven't changed the search controller in any way, other than how to get the data... But it shouldn't be a problem to add later.

This branch isn't supposed to add new features and can be merged alone.

Hows your progress going in #192 going?
Perhaps we should merge that first.
Then this.
And then we can get going on adding new features...

@TUM-Dev TUM-Dev deleted a comment from TCA-Bot Oct 26, 2017
@TCA-Bot
Copy link
Collaborator

TCA-Bot commented Oct 26, 2017

SonarQube analysis reported 4 issues

  • MINOR 1 minor
  • INFO 3 info

Watch the comments in this conversation to review them.

@nerdsupremacist nerdsupremacist merged commit c7d2546 into master Nov 2, 2017
@nerdsupremacist nerdsupremacist deleted the mathias/network-refactor branch November 2, 2017 15:13
@tgymnich tgymnich mentioned this pull request Nov 4, 2017
1 task
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants