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

✨ Export accounts from workspace to csv #184

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

foysalit
Copy link
Contributor

For record keeping, sharing data for further research and fill legal requests exporting data of a group of users is often done manually by mods. This is aimed to make that process much easier.

Comment on lines +156 to +159
repo.email || 'Unknown',
`${repo.ip || 'Unknown'}`,
`${profile?.displayName || 'Unknown'}`,
repo.labels?.map(({ val }) => val).join(', ') || 'None',
Copy link
Collaborator

Choose a reason for hiding this comment

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

I almost wonder if an empty value could be more useful than a default value here. I suppose it depends how the CSV will be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I think our current use case is for mod folks to get these exports and review manually in which case, I feel like the explicit default values are more helpful.

repo.labels?.map(({ val }) => val).join(', ') || 'None',
buildBlueSkyAppUrl({ did: repo.did }),
]
return line.join(',')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Values containing commas or newlines will end up breaking the CSV format. If we add some escaping we should be able to address it, though:

const escapeCSVValue = (value: string) => {
  if (value.contains(',') || value.contains('"') || value.contains('\r') || value.contains('\n')) {
    return `"${value.replaceAll('"', '""')}"`
  }
  return value
}

// ...

  return line.map(escapeCSVValue).join(',')

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.

3 participants