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

Admin features frontend #629

Merged
merged 9 commits into from
Dec 19, 2023
Merged

Admin features frontend #629

merged 9 commits into from
Dec 19, 2023

Conversation

Brunettow
Copy link
Contributor

Since some of the features implemented in this branch is needed by others, It will be merged to the frontend.

So far admin feature buttons are implemented.
Visibility of the buttons are adjusted however not connected to the backend.
Only basic user provider is used in profile page to get user type.

@Brunettow Brunettow added priority: critical This issue requires immediate attention to prevent major issues or disruptions. size: medium Requires moderate effort and may take several days to a week to complete. difficulty: moderate Requires more than basic skills and knowledge, and may require research or collaboration to complete frontend labels Dec 18, 2023
@Brunettow Brunettow linked an issue Dec 18, 2023 that may be closed by this pull request
20 tasks
@Brunettow Brunettow changed the base branch from main to frontend December 18, 2023 17:07
@laylaylo laylaylo self-requested a review December 18, 2023 18:23
@laylaylo
Copy link
Contributor

Now, without even login, I can see admin buttons on node page. Is this intended implementation?
If user type provider is connected and getting the type, why it is not used? Is it currently only for basic user, I didn't understand that part totally.

Copy link
Contributor

@laylaylo laylaylo left a comment

Choose a reason for hiding this comment

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

I think there is a bug about seeing profile page in this branch, because it works fine on frontend branch. It's not possible to visit profile pages as a guest (without logging). Can you please check it out?

@override
Widget build(BuildContext context) {
final User? user = Provider.of<Auth>(context).user;
if (user == null) {
// guest can see profile pages
// TODO guest can see profile pages
Copy link
Contributor

Choose a reason for hiding this comment

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

oh I see that it's not a bug but a whole logic change here, need to understand why this is all changed. Could you please briefly summarize?

Copy link
Contributor

Choose a reason for hiding this comment

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

After a discussion, we realized that there was a misunderstanding about these comments. Please delete TODO parts.

@Brunettow
Copy link
Contributor Author

Thank you for catching this bug. I fixed and re-uploaded the code.
Now, guest user can see the profile pages without an error.

final String email;
final String name;
final String surname;
final int noWorks;
final String aboutMe;
final bool isBanned;
Copy link
Contributor

Choose a reason for hiding this comment

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

woah pretty complicated and scary constructor. There are always better ways to pass these kind of informations. Just keep in mind not to do that in the future, but for this class as it is used by just one parent widget, that's OK

mainAxisAlignment: MainAxisAlignment.start,
crossAxisAlignment: CrossAxisAlignment.end,
children: [
Visibility(
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice widget use! Visibility widget made everything clear.

@Brunettow
Copy link
Contributor Author

Buttons on node view page are not connected to the backend yet. That's why you can see it unlike profile page. It will be implemented in the next PR. Since some of the implementations are needed by @Zulalm, we need this branch to be merged to the frontend branch.

@Brunettow Brunettow requested a review from laylaylo December 18, 2023 21:05
Copy link
Contributor

@laylaylo laylaylo left a comment

Choose a reason for hiding this comment

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

nice work

@Zulalm Zulalm mentioned this pull request Dec 18, 2023
1 task
@Brunettow Brunettow merged commit f310758 into frontend Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: moderate Requires more than basic skills and knowledge, and may require research or collaboration to complete frontend priority: critical This issue requires immediate attention to prevent major issues or disruptions. size: medium Requires moderate effort and may take several days to a week to complete.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FE - Admin Features
4 participants