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

Fixed starter issue: CSV file headers. #762

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

Conversation

kotrynav
Copy link
Member

#734 This issue prevented errors when headers in CSV files were present. Also added HTML error messages on failed uploads to specify which rows could not be matched to existing schools or committees.

Copy link
Contributor

@mathildepm mathildepm left a comment

Choose a reason for hiding this comment

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

LGTM!
Idk why some tests are running so we'll see when you push again.
Let's revert committee.py to what it was before since we aren't worried about that:
https://dev.to/lofiandcode/git-and-github-how-to-revert-a-single-file-dha

failed_uploads.append(row)
if failed_uploads:
messages.error(
request, 'Not all secretariat members could upload. These rows failed: '
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
request, 'Not all secretariat members could upload. These rows failed: '
request, 'Not all Committees could upload. These rows failed: '

Copy link
Contributor

Choose a reason for hiding this comment

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

JK let's just revert this file

@mathildepm
Copy link
Contributor

ALSO; let's update the PR description so it describes more accurately what the PR is doing

  • Reduces errors due to CSV headers
  • Prevents entire failures of uploads if a single row fails - instead tracks failed rows.

@mathildepm
Copy link
Contributor

We were looking through this code for huxley notes app and realized that we should also be validating emails in load so that should be a todo.

@srisainachuri
Copy link
Contributor

srisainachuri commented Nov 28, 2021

Here's a reference to the commit in huxley notes app in delegate.py's load method for the email check:
9d9859f

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