Skip to content

Feature user register#7

Closed
Vasilijez wants to merge 26 commits into
developfrom
feature-user-register
Closed

Feature user register#7
Vasilijez wants to merge 26 commits into
developfrom
feature-user-register

Conversation

@Vasilijez

@Vasilijez Vasilijez commented Jul 26, 2025

Copy link
Copy Markdown
Member

I have introduced the following project folder structure. It is a little overcomplex, but I couldn’t overthink it anymore. If you don’t like this organization, I am ready to make changes. I remain flexible to make any necessary adjustments.

Files like handler.go, route.go, service.go, repo.go can become folders and be split into multiple files such as auth_service.go and user_service.go. But they can also stay as they are, thanks to the simplicity of our microservice.

The middleware folder can be populated with components such as logging, metrics, and others.

The config folder can contain all configuration files, like database settings and others.

I started writing tests in the test folder based on feature names, as this seemed like the best approach from my point of view.


I don’t have much to say about implementing user registration. It is CRUD.

I attempted to implement centralized error handling using error_middleware.go. It will intercept all HTTP requests and add HTTP status codes based on the errors. It also centralizes log data. For customized errors, it is handy to sometimes wrap them with additional data using fmt.Errorf.

magley and others added 21 commits July 21, 2025 13:31
Exported variables should be named using PascalCase.

Since I used `GORM` for modeling, I need a database connection. It uses `sql.DB` under the hood, so when I close `sql.DB`, I automatically close the `GORM` connection as well.
This is quite a large commit—sorry about that! That said, most of the changes are related to restructuring the folder layout, which I believe is a crucial improvement.

I’ve decided to stop overthinking the project structure and went with something scalable, even if it's a bit overkill for now. I’ll explain more in the PR description.

Also, I’ve introduced error handling via middleware.
- I also tried to improve error handling.
- Typo: `ErrUserExists` → `ErrUsernameExists`
@Vasilijez
Vasilijez requested a review from magley July 26, 2025 10:53
@Vasilijez Vasilijez self-assigned this Jul 26, 2025
@Vasilijez Vasilijez added the feature New functional or non-functional requirement implementation. label Jul 26, 2025
@magley

magley commented Jul 26, 2025

Copy link
Copy Markdown
Member

Why are my commits part of this PR? Did you do this e156f37 manually?

@Vasilijez

Vasilijez commented Jul 26, 2025

Copy link
Copy Markdown
Member Author

Honestly, I don't know, maybe I accidentally created the feature-user-register branch based on PR #2 instead of develop, as I usually do.

@magley magley left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As far as the project structure goes, I think it's overengineered. There's no need to package by layer for such a small service. Also, do we really need a struct for each layer? I would've written everything as a function. So, registering users would be HandleRegister(), Register(), DBRegister(). I think that's closer to idiomatic Go. We don't want another MockerHub that's trying to be like Spring Boot.

It doesn't matter. Don't change it now, we would just be wasting time. I want to finish everything ASAP.

I didn't test it manually yet because we have no frontend nor a simple python script (like a import requests; requests.post("...", {...}), and the Address model is needlessly complex.

Comment thread src/main.go Outdated
Comment thread src/domain/user.go Outdated
Comment thread src/domain/address.go Outdated
Comment thread src/api/route.go Outdated
Comment thread src/api/middleware/error_middleware.go Outdated
We could add a backoff, but I think it's smarter to keep retrying indefinitely without delay, since we want all services to start as soon as possible.
I made this change based on our specification, as we likely won’t need more than one service or repository.
I was unsure about the `Register` handler and decided to make it lowercase, as I currently don’t see a reason to export it. Integration tests will likely be performed via API calls.
@Vasilijez
Vasilijez requested a review from magley July 26, 2025 15:50
@Vasilijez
Vasilijez changed the base branch from develop to master July 27, 2025 08:04
@Vasilijez
Vasilijez changed the base branch from master to develop July 27, 2025 08:04
@Vasilijez

Copy link
Copy Markdown
Member Author

I applied the following method for resolving the rebase problem.

I created the feature-user-register-rebase branch and then used the git cherry-pick operation to copy my commits from the feature-user-register branch. This way, I avoided having @magley’s overlapping commits from the develop branch show up in the PR.

All previous commits remained unchanged and have the same hash values. All the commits I copied have new hash values. The feature-user-register branch will not be merged.

If I went with the rebase operation, it’s very likely that even @magley’s commits would get new hash values, and who knows what else would happen, since it would be necessary to go back through history and resolve conflicts, which already appear from the very first commit. That’s a very messy solution, and that’s why I’m trying to avoid handling it that way.

The simplest methods, like changing the base branch from develop to master and back, didn’t work, this was the next simplest approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New functional or non-functional requirement implementation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants