Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bf 17 account page #65

Merged
merged 31 commits into from
Dec 30, 2022
Merged

Bf 17 account page #65

merged 31 commits into from
Dec 30, 2022

Conversation

AbdelrahmanMosly
Copy link
Collaborator

@AbdelrahmanMosly AbdelrahmanMosly commented Dec 28, 2022

Front-End

  • added user_profile_api to send request for user profile
  • added modle User profile to map request to it
  • created profile page
  • edited custom field text to be able to take attribute enabled default = true and not required
  • used it to make profile page fields

Back-End

  • added userProfile classs to collect data from DB to it and send it to back
  • created get request to get userProfile class
  • added new request to security
    image
    image

@AbdelrahmanMosly AbdelrahmanMosly temporarily deployed to Production December 28, 2022 22:09 — with GitHub Actions Inactive
@AbdelrahmanMosly AbdelrahmanMosly temporarily deployed to Production December 28, 2022 22:12 — with GitHub Actions Inactive
@AbdelrahmanMosly AbdelrahmanMosly temporarily deployed to Production December 28, 2022 22:18 — with GitHub Actions Inactive
@MuhammadKotb MuhammadKotb added the enhancement New feature or request label Dec 29, 2022
@MuhammadKotb MuhammadKotb added this to the Milestone 3 milestone Dec 29, 2022
@AbdelrahmanMosly AbdelrahmanMosly temporarily deployed to Production December 29, 2022 21:46 — with GitHub Actions Inactive
@AbdelrahmanMosly AbdelrahmanMosly temporarily deployed to Production December 29, 2022 23:04 — with GitHub Actions Inactive
@AbdelrahmanMosly AbdelrahmanMosly temporarily deployed to Production December 29, 2022 23:25 — with GitHub Actions Inactive
@AbdelrahmanMosly AbdelrahmanMosly temporarily deployed to Production December 29, 2022 23:40 — with GitHub Actions Inactive
@AbdelrahmanMosly AbdelrahmanMosly temporarily deployed to Production December 29, 2022 23:46 — with GitHub Actions Inactive
@AbdelrahmanMosly AbdelrahmanMosly temporarily deployed to Production December 30, 2022 09:31 — with GitHub Actions Inactive
@AbdelrahmanMosly AbdelrahmanMosly temporarily deployed to Production December 30, 2022 09:32 — with GitHub Actions Inactive
@AbdelrahmanMosly AbdelrahmanMosly added the refactoring Improve the design, structure, implementation and increase code readability and reduced complexity label Dec 30, 2022
@AbdelrahmanMosly AbdelrahmanMosly temporarily deployed to Production December 30, 2022 10:17 — with GitHub Actions Inactive
@AbdelrahmanMosly AbdelrahmanMosly temporarily deployed to Production December 30, 2022 15:03 — with GitHub Actions Inactive
@RequestMapping("account_settings")
public class UserProfileController {

@Autowired
Copy link
Member

@Deffo0 Deffo0 Dec 30, 2022

Choose a reason for hiding this comment

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

Autowired here isn't necessary, as you can replace it by constructor injection which has alot of advantages as following:

  • the dependencies are clearly identified. There is no way to forget one when testing, or instantiating the object in any other circumstance (like creating the bean instance explicitly in a config class).

  • the dependencies can be final, which helps with robustness and thread-safety.

  • you don't need reflection to set the dependencies. InjectMocks is still usable, but not necessary. you can just create mocks by yourself and inject them by simply calling the constructor.

  • by the autowired approach, you are allowing anyone (in different class outside/inside the Spring container) to create an instance using default constructor (like new SomeService()), which is NOT good as you need SomeOtherService object (as a dependency) for your SomeService.

You may have some questions like "is this is applicable in the stereo type component as you don't call it?"

The answer is yes and you can do it even for JPA repositories.

how does spring inject the dependency in this case ?

Spring scans your classes for constructor that matches your class' fields. Find details here:
Additional Details

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wouldn't prefer Autowired my self as well here but I decided to use it to be consistent with the rest of the project.

Copy link
Member

Choose a reason for hiding this comment

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

Good, so we should mark it as an issue. I think it will need a huge refactoring, but it will be worthy.

var url = Uri.https(APIConstants.baseUrl, APIConstants.userProfileEndPoint);
var response = await http.get(
url, headers: APIConstants.headerCORS(session.cookie));
debugPrint(url.toString());
Copy link
Member

Choose a reason for hiding this comment

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

I think, those debugging prints should be removed if you are finished.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

you are right I forgot it.

frontend/lib/screens/account_settings.dart Show resolved Hide resolved
@AbdelrahmanMosly AbdelrahmanMosly temporarily deployed to Production December 30, 2022 15:58 — with GitHub Actions Inactive
@AbdelrahmanMosly AbdelrahmanMosly merged commit 3fd415e into Milestone3 Dec 30, 2022
@Deffo0 Deffo0 mentioned this pull request Dec 30, 2022
@AbdelrahmanMosly AbdelrahmanMosly mentioned this pull request Jan 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request refactoring Improve the design, structure, implementation and increase code readability and reduced complexity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants