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

Custom export format #549

Merged
merged 59 commits into from
Sep 23, 2022
Merged

Custom export format #549

merged 59 commits into from
Sep 23, 2022

Conversation

nebs-dev
Copy link
Contributor

@nebs-dev nebs-dev commented Aug 15, 2022

Resolves #532

Added:

  • new export options for the period token distribution
  • new settings section for custom export
  • api endpoint /exportSummary that i using export data transformer from any external url
  • refactor of settings form

@nebs-dev nebs-dev requested a review from kristoferlund August 15, 2022 16:17
@nebs-dev nebs-dev self-assigned this Aug 15, 2022
@@ -295,3 +300,50 @@ export const isPeriodLatest = async (

return false;
};

export const getSummarizedReceiverData = (
data: PeriodDetailsGiverReceiverDto[] | undefined,
Copy link
Member

Choose a reason for hiding this comment

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

Try to avoid funcftions accepting undefined if possible. Passing undefinedto a function only passes the "problem" to somewhere else in the code.

on: operateItem.on,
};
}),
// eslint-disable-next-line @typescript-eslint/no-implied-eval
Copy link
Member

Choose a reason for hiding this comment

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

I simplefied this part and reworked implementation in jsonTransformer slightly.

@@ -55,6 +57,14 @@ const FormFields = (
setting.valueRealized as string,
disabled
);
else if (setting.type === 'Radio')
Copy link
Member

Choose a reason for hiding this comment

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

We cannot assume these are the two only possible values. Along with the settings definition we would need to store possible values. If this seems like too much work for now, just go with a plain string.

@@ -53,6 +56,10 @@ const SettingsPage = (): JSX.Element | null => {
<NavItem to={`${url}/application`} description="Application" />
<NavItem to={`${url}/period`} description="Period Defaults" />
<NavItem to={`${url}/discord`} description="Discord Bot" />
<NavItem
Copy link
Member

Choose a reason for hiding this comment

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

The cusom export settings page doesn't render properly here.

"Rendered more hooks than during the previous render."

@@ -39,9 +39,11 @@
"env-cmd": "^10.1.0",
"ethers": "^5.6.8",
"final-form": "^4.20.4",
"js-base64": "^3.7.2",
Copy link
Member

Choose a reason for hiding this comment

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

Currently unused.

@@ -124,3 +125,9 @@ export interface ReplaceQuantifierRequestBody {
currentQuantifierId: string;
newQuantifierId: string;
}

export interface SummarizedPeriodData {
Copy link
Member

Choose a reason for hiding this comment

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

This represents one interface for one custom export only. There will be many export types defined by .. whoever. There will be no export intefaces since everything is based just on the transformer json format.

@@ -89,6 +90,11 @@ export function isSettingValueAllowedBySettingType(
return valid;
}

if (this.type === 'Object') {
Copy link
Member

Choose a reason for hiding this comment

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

It is not object but JSON, right?

@@ -58,6 +60,7 @@
"recoil": "^0.7.4",
"recoil-nexus": "^0.4.0",
"recoil-persist": "^4.2.0",
"safe-eval": "^0.4.1",
Copy link
Member

Choose a reason for hiding this comment

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

Do safe-eval need to be imported in frontend?

@nebs-dev nebs-dev changed the title WIP: Custom export format Custom export format Aug 31, 2022
@nebs-dev nebs-dev marked this pull request as ready for review September 1, 2022 08:23
Comment on lines 537 to 550
const fields = [
{
label: 'ADDRESS',
value: 'address',
},
{
label: 'AMOUNT',
value: 'amount',
},
{
label: 'TOKEN',
value: 'tokenName',
},
];
Copy link
Member

@kristoferlund kristoferlund Aug 31, 2022

Choose a reason for hiding this comment

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

Export should not assume these are the fields to export. Instead use the information in the transformer json: map.item.

Comment on lines 154 to 156
handleExport();
} else if (option.value === 'export-summary') {
handleExport();
Copy link
Member

Choose a reason for hiding this comment

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

export-full and export-summary should not be the same.

if (!period) return undefined;

const response = await apiAuthClient.get(
`/admin/periods/${period._id}/exportSummary?context=${exportContext}&supportPercentage=${supportPercentage}`,
Copy link
Member

Choose a reason for hiding this comment

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

/exportSummary returns summarised praise data for the period, basically the input to the custom export
New endpoint: /customExport does what /exportSummary currently does.

Co-authored-by: Nebs <nebojsa.stojanovic0@gmail.com>
@kristoferlund
Copy link
Member

@nebs-dev remove checkbox in the export dialog and instead just print message.

If CS support percentage > 0:

Thank your for supporting the continued development of Praise, 2% of the distribution will be donated to the Praise dev team.

If CS support percentage = 0:

Consider supporting the development of Praise by donating a small amount of the distribution to the Praise dev team. Settings: Custom token export

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 7 Code Smells

No Coverage information No Coverage information
5.1% 5.1% Duplication

Copy link
Member

@kristoferlund kristoferlund left a comment

Choose a reason for hiding this comment

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

Great work with this issue Nebs! Merging now.

@kristoferlund kristoferlund merged commit 6d120e7 into main Sep 23, 2022
@kristoferlund kristoferlund deleted the enh/custom_export_formats branch September 26, 2022 08:55
@kristoferlund kristoferlund mentioned this pull request Sep 26, 2022
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.

Custom export formats
2 participants