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

Dashboard page #184

Open
wants to merge 34 commits into
base: develop
Choose a base branch
from
Open

Dashboard page #184

wants to merge 34 commits into from

Conversation

mkmak2
Copy link
Contributor

@mkmak2 mkmak2 commented Jan 7, 2023

Ticket

Dashboard Page

Summary

to do:
show how many unread comments u have,
fix page display in navbar,
clean code

@mkmak2 mkmak2 requested a review from cyp3rius January 7, 2023 21:13
@mkmak2 mkmak2 changed the base branch from master to develop January 7, 2023 21:13
@codecov-commenter
Copy link

codecov-commenter commented Jan 7, 2023

Codecov Report

Merging #184 (a8fa17b) into develop (34faafa) will decrease coverage by 1.60%.
The diff coverage is 0.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@             Coverage Diff             @@
##           develop     #184      +/-   ##
===========================================
- Coverage    79.39%   77.80%   -1.60%     
===========================================
  Files           31       31              
  Lines          830      847      +17     
  Branches       276      279       +3     
===========================================
  Hits           659      659              
- Misses         168      185      +17     
  Partials         3        3              
Flag Coverage Δ
unittest 77.80% <0.00%> (-1.60%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
content-types/comment.ts 100.00% <ø> (ø)
server/services/admin.ts 70.20% <0.00%> (-6.60%) ⬇️
server/services/common.ts 79.65% <ø> (ø)

@@ -18,14 +18,16 @@ import {
useNotification,
} from '@strapi/helper-plugin';

import ComingSoonPage from '../ComingSoonPage';
//import ComingSoonPage from '../ComingSoonPage';
Copy link
Contributor

Choose a reason for hiding this comment

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

left over?

Comment on lines +170 to +172



Copy link
Contributor

Choose a reason for hiding this comment

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

Too much spacing left

Comment on lines +127 to +129

console.log(result)

Copy link
Contributor

Choose a reason for hiding this comment

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

left over

Comment on lines +98 to +99
console.log(result);

Copy link
Contributor

Choose a reason for hiding this comment

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

left over

@@ -81,12 +81,12 @@ const Details = ({ config }) => {
parseRegExp(config.regex.uid).flags
);

const {
const {
Copy link
Contributor

Choose a reason for hiding this comment

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

redundant spacing

Comment on lines +235 to +246
</Table>

) : (
<EmptyStateLayout content={emptyLayout[emptyContent]} />
)}
</PanelLayout>
</>
)}
</Layout>
) :
<NoAcccessPage />;

Copy link
Contributor

Choose a reason for hiding this comment

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

Redundant empty lines left

Comment on lines 108 to 114



Copy link
Contributor

Choose a reason for hiding this comment

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

Same

push(getUrl(`discover/${id}`));
};

const isLoading = isLoadingForData || isFetching;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const isLoading = isLoadingForData || isFetching;
const isLoading = isLoadingForData || isFetching || isLoadingForPermissions;

To make compound flag, well, compound and in one place.

@kamilszewczyk0 kamilszewczyk0 self-requested a review November 25, 2023 14:21
Copy link
Contributor

@kamilszewczyk0 kamilszewczyk0 left a comment

Choose a reason for hiding this comment

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

I cannot display comments, neither on dashboard, nor discover. Also, error on toast is translate key, not string with error message ;/

Comment on lines +16 to +23

position: absolute;
right: -30%;
top: -30%;
right: -20%;
top: -20%;

border-radius: 50%;

background: transparent;
Copy link
Contributor

Choose a reason for hiding this comment

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

unnecessary empty spaces

)}
/>
)}
{!isAdminComment && (<>
Copy link
Contributor

Choose a reason for hiding this comment

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

would You consider extracting this fragment to separate component? Also, my recommendation is to chop this component to smaller parts, which will make it more readable

@@ -30,7 +30,7 @@ export const CustomLinkIconButton = styled(CustomIconButton)`
`;

export const MainButtons = styled(IconButtonGroup)`
margin-left: ${({ theme }:ThemeInterface) => theme.spaces[4]};

`;

export const MoreButton = styled(IconButton)`
Copy link
Contributor

Choose a reason for hiding this comment

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

component not used

Comment on lines +47 to +75
useFocusWhenNavigate();

const {push} = useHistory();
const {notifyStatus} = useNotifyAT();
const {trackUsage} = useTracking();
const trackUsageRef = useRef(trackUsage);
const toggleNotification = useNotification();
const [{query: queryParams}] = useQueryParams();
const _q = queryParams?._q || '';

const viewPermissions = useMemo(
() => ({
access: pluginPermissions.access,
moderate: pluginPermissions.moderate,
accessReports: pluginPermissions.reports,
reviewReports: pluginPermissions.reportsReview,
}),
[],
);

const {
isLoading: isLoadingForPermissions,
allowedActions: {
canAccess,
canModerate,
canAccessReports,
canReviewReports,
},
} = useRBAC(viewPermissions);
Copy link
Contributor

Choose a reason for hiding this comment

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

code repetition - would You consider to extract this to separate file?

Comment on lines +70 to +83
const [storedReports, setStoredReports] = useState([]);
const [selectedReports, setSelectedReports] = useState([]);

useFocusWhenNavigate();

const { push } = useHistory();
const { notifyStatus } = useNotifyAT();
const { trackUsage } = useTracking();
const trackUsageRef = useRef(trackUsage);
const toggleNotification = useNotification();
const [{ query: queryParams }] = useQueryParams();
const _q = queryParams?._q || "";
const queryClient = useQueryClient();
const { lockApp, unlockApp } = useOverlayBlocker();
Copy link
Contributor

Choose a reason for hiding this comment

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

code repetition - would You consider to extract this to separate file?

Comment on lines +100 to +167
const onError = (err) => {
handleAPIError(err, toggleNotification);
};

const onSelectionChange = (selection) => setSelectedReports(selection);

const handleItemSelectionChange = (selection, value) => {
if (isArray(selection)) {
onSelectionChange(value ? selection : []);
} else {
onSelectionChange(
[...selectedReports, selection].filter(
(item) => value || selection !== item,
),
);
}
};

const areAllItemsSelected = () =>
!isEmpty(selectedReports)
? selectedReports.length === storedReports.length
: false;

const isItemSelected = (id) => selectedReports.includes(id);

const hasAnySelectedReports = selectedReports.length > 0;

const onValueChange = useCallback(
(value) => {
handleItemSelectionChange(
storedReports.map((_) => _.id),
value,
);
},
[storedReports],
);

const resolveReportMutation = useMutation(resolveReport, {
onSuccess: onSuccess(),
onError,
refetchActive: false,
});

const resolveMultipleReportsMutation = useMutation(resolveMultipleReports, {
onSuccess: onSuccess(),
onError,
refetchActive: false,
});

const handleClickResolveSelected = async () => {
if (canReviewReports) {
lockApp();
const items = await resolveMultipleReportsMutation.mutateAsync(
selectedReports,
);
if (!isEmpty(items)) {
const updatedReports = storedReports.map((_) => ({
..._,
resolved: selectedReports.includes(_.id) ? true : _.resolved,
}));
setStoredReports(updatedReports);
setSelectedReports([], false);
}
}
};

const isLoading = isLoadingForData || isFetching;
const { total } = pagination;
Copy link
Contributor

Choose a reason for hiding this comment

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

another repetition - it would be better to place it somewhere else and reuse it

Comment on lines +170 to +172



Copy link
Contributor

Choose a reason for hiding this comment

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

please do not leave more than 1 empty line of space

@@ -26,6 +26,9 @@ import { IconButtonGroupStyled } from "../../../../components/IconButton/styles"
import DiscussionThreadItemReviewAction from "../../../../components/DiscussionThreadItemReviewAction";
import UserAvatar from "../../../../components/Avatar";

Copy link
Contributor

Choose a reason for hiding this comment

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

unnecessary empty space

Comment on lines +113 to 115


const {
Copy link
Contributor

Choose a reason for hiding this comment

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

please leave just one line of space

@@ -289,6 +289,7 @@ const controllers: IControllerAdmin = {
}
},


Copy link
Contributor

Choose a reason for hiding this comment

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

unnecessary empty line

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants