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

feat: improve import file to calendar and extend capabilities #201

Merged

Conversation

Nir-P
Copy link
Contributor

@Nir-P Nir-P commented Feb 4, 2021

This is an extension for import to calendar:

what's new?

  • Support "Location" field when importing txt, csv or ics files
    files must be in the following structure or the old structure (without Location)
    Title1, Content1, S_Date1, E_Date1, Location1
    Title2, Content2, S_Date2, E_Date2, Location2
    ...
    TitleN, ContentN, S_DateN, E_DateN, LocationN
    
  • Start and End dates support hours and minutes format

the new rules we set (can be changed through a global variable):

  • The location field length is up to 50 characters.
  • The event duration limit.

Additional tests added:

  • Start date is smaller or equal to the end date.
  • The Location field may be empty.
  • Checking that new files from the old and new format are working flawlessly.
  • And some more here and there :)

Copy link
Member

@yammesicka yammesicka left a comment

Choose a reason for hiding this comment

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

Great job! Added few of my insights :)

check_date = datetime.datetime.strptime(date, DATE_FORMAT)
except ValueError:
check_date = change_string_to_date(date)
if check_date is False:
Copy link
Member

Choose a reason for hiding this comment

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

Prefer == False

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed it to not if this is ok.

app/internal/import_file.py Outdated Show resolved Hide resolved
date1 = datetime.datetime.strptime(date, DATE_FORMAT2)
else:
date1 = datetime.datetime.strptime(date, DATE_FORMAT)
except ValueError:
Copy link
Member

Choose a reason for hiding this comment

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

What is the purpose of the except here?

Copy link
Contributor Author

@Nir-P Nir-P Feb 5, 2021

Choose a reason for hiding this comment

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

if the string is not a valid date datetime.strptime will fail to transform the string to date and we want to catch it.

@codecov-io
Copy link

codecov-io commented Feb 5, 2021

Codecov Report

Merging #201 (9a4124e) into develop (a1f2998) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #201      +/-   ##
===========================================
+ Coverage    99.36%   99.39%   +0.02%     
===========================================
  Files           36       37       +1     
  Lines         1418     1485      +67     
===========================================
+ Hits          1409     1476      +67     
  Misses           9        9              
Impacted Files Coverage Δ
app/internal/import_file.py 100.00% <100.00%> (ø)
app/routers/dayview.py 100.00% <0.00%> (ø)
app/internal/quotes/daily_quotes.py
app/internal/quotes/load_quotes.py
app/internal/zodiac.py 100.00% <0.00%> (ø)
app/internal/json_data_loader.py 100.00% <0.00%> (ø)
app/internal/daily_quotes.py 100.00% <0.00%> (ø)
app/database/models.py 96.22% <0.00%> (+0.39%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a1f2998...9a4124e. Read the comment docs.

@Nir-P Nir-P requested a review from yammesicka February 5, 2021 13:57
@yammesicka yammesicka merged commit 063092e into PythonFreeCourse:develop Feb 10, 2021
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.

None yet

3 participants