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

Delete events not found in Elis in kulke importer #625

Merged
merged 6 commits into from
Jul 26, 2023
Merged

Conversation

tuhola
Copy link
Contributor

@tuhola tuhola commented Jun 29, 2023

If there are events that have previously been imported, but have since been removed in Elis, there's currently no mechanism to remove them from LE. This PR adds such a mechanism. Basically we check for any events that we have previously imported that no longer exist in Elis. We won't touch events that are older than the start date we use for the Elis search though.

Also, the fix for the empty name issue in my previous PR was not sufficient, so I fixed that. The logic for taking the common part when creating a super event name should now work in all supported languages.

When reviewing, please pay special attention to the logic for selecting which events to delete.

@codecov-commenter
Copy link

codecov-commenter commented Jun 30, 2023

Codecov Report

Merging #625 (7e237d2) into develop (cbab993) will increase coverage by 0.78%.
The diff coverage is 96.92%.

@@             Coverage Diff             @@
##           develop     #625      +/-   ##
===========================================
+ Coverage    74.52%   75.31%   +0.78%     
===========================================
  Files          237      237              
  Lines        17074    17178     +104     
===========================================
+ Hits         12725    12937     +212     
+ Misses        4349     4241     -108     
Files Changed Coverage Δ
events/importer/kulke.py 37.50% <87.50%> (+17.80%) ⬆️
events/models.py 91.52% <100.00%> (+0.02%) ⬆️
events/tests/factories.py 100.00% <100.00%> (ø)
events/tests/importers/test_kulke.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

events/tests/factories.py Outdated Show resolved Hide resolved
events/importer/kulke.py Outdated Show resolved Hide resolved
events/importer/kulke.py Outdated Show resolved Hide resolved
events/importer/kulke.py Outdated Show resolved Hide resolved
events/importer/kulke.py Outdated Show resolved Hide resolved
@danipran danipran force-pushed the kulke-deletion branch 2 times, most recently from 30493e1 to 5fc016f Compare July 18, 2023 12:24
Copy link
Contributor

@charn charn left a comment

Choose a reason for hiding this comment

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

Couldn't spot anything serious. 👍 :shipit: A minor comment about testing those soft deleted events.

@sonarcloud
Copy link

sonarcloud bot commented Jul 26, 2023

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 0 Code Smells

91.2% 91.2% Coverage
0.0% 0.0% Duplication

@danipran danipran dismissed their stale review July 26, 2023 13:49

Ended up doing the changes myself, so approving my own changes isn't exactly cool.

@danipran danipran merged commit cdccac1 into develop Jul 26, 2023
3 checks passed
@danipran danipran deleted the kulke-deletion branch July 26, 2023 13:51
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.

4 participants