User registration for new exam target #312

ivan-magda · 2018-07-04T18:30:19Z

Задача: #APPS-1940

Коротко для Release Notes, в формате «Сделали/Добавили/Исправили N»:

Создан новый модуль ExamEGERussian для приложения экзаменов по русскому языку.
Добавлены необходимые файлы в Compile Sources для работы API.
При старте происходит регистрация пользователя с фейковым email и отписка от рассылок. (Используется реализация из адаптивного модуля)

Описание:
Анонимная авторизация пользователя в модули для экзаменов по русскому языку.

Сорян, неудобно будет проверять, нужно было разбить на несколько PR :)

Add compile sources files to the ExamEGERussian target to be able to use Stepic API's.

Create: - Auth.plist - holds own the keys for performing API requests - Config.plist - contains API's endpoints URL's

Create: - UserRegistrationService - for anonymous user registration and login - UserSubscriptionsService - for managing user subscriptions preferences

Create RootNavigationManager and assembly for services.

kvld · 2018-07-05T09:58:30Z

@vanyaland в dev ветке обновился PromiseKit, подмерджи себе

kvld

Всё очень круто! Давай обсудим/поправим и вмерджим.

kvld · 2018-07-05T11:53:41Z

...rces/BusinessLogicLayer/Assemblies/ServiceComponentsAssembly/ServiceComponentsAssembly.swift

+
+final class ServiceComponentsAssembly: ServiceComponents {
+
+    // MARK: Object Initialization


Когда код достаточно декомпозирован, нужды в MARK: особо нет (к тому же, не все используют XCode). Но это ИМХО и если привык и считаешь, что это нужно – ок.

kvld · 2018-07-05T11:55:29Z

...rces/BusinessLogicLayer/Assemblies/ServiceComponentsAssembly/ServiceComponentsAssembly.swift

+
+    // MARK: - UserRegistrationService
+
+    let userRegistrationService: UserRegistrationService


Мы выносим поля перед конструкторами и остальными методами.

kvld · 2018-07-05T11:59:24Z

...an/Sources/BusinessLogicLayer/Services/UserRegistrationService/UserRegistrationService.swift

+// MARK: UserRegistrationServiceError: Error
+
+enum UserRegistrationServiceError: Error {
+    case userNotRegistered


Если здесь все ошибки будут с префиксом user-, то имеет смысл его опустить.

kvld · 2018-07-05T12:00:34Z

...inessLogicLayer/Services/UserRegistrationService/UserRegistrationServiceImplementation.swift

+
+    let stepicsAPI: StepicsAPI
+
+    func registerNewUser() -> Promise<User> {


Было бы круто шарить этот код с адаптивными, но можно перенести его сюда, а потом заюзать его там.

kvld · 2018-07-05T12:03:58Z

ExamEGERussian/Sources/PresentationLayer/Base.lproj/Main.storyboard

@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="UTF-8"?>


Раз уж начинаем с чистого листа, то стоит подумать перед тем, как юзать сториборды. Возможно, лучше отдельные xibы для каждого контроллера.

kvld · 2018-07-05T12:06:43Z

.../Sources/BusinessLogicLayer/Services/UserSubscriptionsService/UserSubscriptionsService.swift

+
+// MARK: - UserSubscriptionsService -
+
+protocol UserSubscriptionsService {


Пока непонятно, зачем размазывать похожую логику на два сервиса.
Если бы у нас была отдельная работа с профилями, то имело бы смысл отделить её от работы с пользователями, но тут, кажется, не планируется ничего кроме отписки.

kvld · 2018-07-05T12:10:54Z

ExamEGERussian/Sources/PresentationLayer/UserStories/Main/MainViewController.swift

+
+    // MARK: - Instance Properties
+
+    var userRegistrationService: UserRegistrationService?


Надеюсь, это для дебага и потом здесь добавится отдельная абстракция между view и бизнес логикой.

kvld · 2018-07-05T12:12:10Z

ExamEGERussian/SupportingFiles/Auth.plist

+	<key>password</key>
+	<dict>
+		<key>id</key>
+		<string>m3uu1NAGnAGFDHRf4S2Wvg1megAjuqwBsX0OUDru</string>


Это надо закрывать под git-crypt.

kvld · 2018-07-05T12:14:54Z

ExamEGERussianTests/LogicTests/UserRegistrationServiceTests/UserRegistrationServiceTests.swift

+
+        let predicate = NSPredicate(format: "isAuthorized == %@", NSNumber(value: true))
+        let exp = expectation(for: predicate, evaluatedWith: self, handler: nil)
+        let result = XCTWaiter.wait(for: [exp], timeout: 5.0)


Если пишешь тесты с использованием реального API, то есть два варианта:

замокать API

отделять их от остальных тестов, потому что на CI они могут отвалиться с таймаутом

могут отвалиться

обязательно отвалятся, поправил тебя

прерывная интеграция

Ostrenkiy

Мне нравится. Тему с Assembly слоем (который не является чем-то необходимым, как по мне) хотели имплементить в основном приложении еще прошлой осенью, но после обсуждений и споров решили, что это будет довольно сложно. Классно, если в новом проекте это будет реализовано. Предлагаю с осторожностью относиться к выделению большого количества слоев, так как время на разработку ограничено и излишняя декомпозиция зачастую усложняет восприятие кода.

А вообще Влад очень много полезных комментариев оставил, мне в принципе особо нечего добавить.

ivan-magda · 2018-07-05T13:09:21Z

@Ostrenkiy, @kvld, спасибо за комментарии. Согласен, возможно я преждевременной оптимизацией занимаюсь и только все усложняю.

Ostrenkiy · 2018-07-05T14:27:13Z

Тесты не хочется сейчас пока поправлять?

ivan-magda · 2018-07-05T14:28:49Z

@Ostrenkiy Хочется, нужно еще поразбираться, т.к. с моками и библиотеками для них не знаком.

ivan-magda · 2018-07-05T14:30:54Z

Отпишусь как будет готово для ревью.

- Delete Main.storyboard file - Create MainViewController.xib file - Create UIViewController from nib instantiation extensions

kvld · 2018-07-05T16:41:23Z

ExamEGERussian/Sources/ApplicationLayer/RootNavigationManager/RootNavigationManager.swift

+    // MARK: Public API
+
+    func setup(with window: UIWindow) {
+        let mainController: MainViewController = MainViewController.fromNib()


MainViewController.fromNib() – это то же самое, что просто MainViewController(), если есть xib с именем MainViewController.xib

Ага, открыл для себя магию 👍

/* The designated initializer. If you subclass UIViewController, you must call the super implementation of this method, even if you aren't using a NIB. (As a convenience, the default init method will do this for you, and specify nil for both of this methods arguments.) In the specified NIB, the File's Owner proxy should have its class set to your view controller subclass, with the view outlet connected to the main view. If you invoke this method with a nil nib name, then this class' -loadView method will attempt to load a NIB whose name is the same as your view controller's class. If no such NIB in fact exists then you must either call -setView: before -view is invoked, or override the -loadView method to set up your views programatically. */ public init(nibName nibNameOrNil: String?, bundle nibBundleOrNil: Bundle?)

…izer

ivan-magda · 2018-07-06T13:34:29Z

@kvld @Ostrenkiy вроде бы все замечания поправил.

ivan-magda added 7 commits July 3, 2018 15:02

Create ExamEGERussian target

e478a8c

Setup pods for ExamEGERussian target

e474a0b

Add target compile sources files

ae0ffb5

Add compile sources files to the ExamEGERussian target to be able to use Stepic API's.

Create configuration files

4756cef

Create: - Auth.plist - holds own the keys for performing API requests - Config.plist - contains API's endpoints URL's

Create user registration services

2fdafb6

Create: - UserRegistrationService - for anonymous user registration and login - UserSubscriptionsService - for managing user subscriptions preferences

Implement demo user registration

3af1b47

Refactor anonymous user registration process

695e3d5

Create RootNavigationManager and assembly for services.

ivan-magda requested review from Ostrenkiy and kvld July 4, 2018 18:30

ivan-magda added the exam label Jul 4, 2018

ivan-magda self-assigned this Jul 4, 2018

kvld added this to the 1.63 milestone Jul 4, 2018

ivan-magda added 3 commits July 5, 2018 11:54

Test ServiceComponents creation

927d555

Test user registration

daf10ee

Test MainViewController instantiation

206c838

ivan-magda added 2 commits July 5, 2018 13:12

Merge remote-tracking branch 'origin/dev' into exam-ege-ru-auth

02a6563

Support PromiseKit 6

3845ab6

kvld suggested changes Jul 5, 2018

View reviewed changes

Ostrenkiy suggested changes Jul 5, 2018

View reviewed changes

ivan-magda added 4 commits July 5, 2018 16:17

Move up ServiceComponentsAssembly's instance properties

42f5582

Refactor rename UserRegistrationServiceError cases

ee9d53f

Move user unregister from email logic to UserRegistrationService

142c450

Encrypt Auth.plist

72ed676

kvld approved these changes Jul 5, 2018

View reviewed changes

Instantiate view controllers programatically

2d1948f

- Delete Main.storyboard file - Create MainViewController.xib file - Create UIViewController from nib instantiation extensions

kvld reviewed Jul 5, 2018

View reviewed changes

ivan-magda added 3 commits July 5, 2018 19:52

Replace UIViewController+fromNib extension usage with default initial…

05b58ff

…izer

Recover tests

f801d24

Share registration with adaptive targets

e0f282f

Ostrenkiy approved these changes Jul 6, 2018

View reviewed changes

ivan-magda merged commit b54ec3b into dev Jul 6, 2018

ivan-magda deleted the exam-ege-ru-auth branch July 6, 2018 15:08

kvld mentioned this pull request Jul 11, 2018

Release 1.63 #324

Merged


		final class ServiceComponentsAssembly: ServiceComponents {

		// MARK: Object Initialization


		// MARK: - UserRegistrationService

		let userRegistrationService: UserRegistrationService


		let stepicsAPI: StepicsAPI

		func registerNewUser() -> Promise<User> {


		// MARK: - UserSubscriptionsService -

		protocol UserSubscriptionsService {


		// MARK: - Instance Properties

		var userRegistrationService: UserRegistrationService?

User registration for new exam target #312

User registration for new exam target #312

Uh oh!

Conversation

ivan-magda commented Jul 4, 2018

Uh oh!

kvld commented Jul 5, 2018

Uh oh!

kvld left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ostrenkiy Jul 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ostrenkiy left a comment

Choose a reason for hiding this comment

Uh oh!

ivan-magda commented Jul 5, 2018

Uh oh!

Ostrenkiy commented Jul 5, 2018

Uh oh!

ivan-magda commented Jul 5, 2018

Uh oh!

ivan-magda commented Jul 5, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ivan-magda commented Jul 6, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Ostrenkiy Jul 5, 2018 •

edited

Loading