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

fix: external sync when there is no platform with passed name #3385

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

arslanashraf7
Copy link
Contributor

@arslanashraf7 arslanashraf7 commented Jan 24, 2025

What are the relevant tickets?

None, Noticecd this issue while working on another ticket.

Description (What does it do?)

Fixes a bug when we run the command with a non-existent platform name (i.e. exists in the platform mapping but doesn't exist in the system). The command would bypass the missing platform in the system, and the sync flag and run right away.

I've separated the errors to be clearer in stating what problem occurred while running the command. 1. Is the database record missing? 2) Is the mapping missing 3) Is the sync off

Screenshots (if appropriate):

  • Desktop screenshots
  • Mobile width screenshots

How can this be tested?

  1. Run the command with a non-existent platform and you should see the platform missing error
  2. Run the command with the existing platform but which is not present in the internal platform mapping, you should see the missing mapping error
  3. Run the command with a platform that exists in the system as well as in the mapping, but disable the platform sync flag through Django admin, you should see the sync disabled error
  4. Run the comment with point#3, except this time enable the platform sync flag, the command should run successfully

Additional Context

@odlbot odlbot temporarily deployed to xpro-ci-pr-3385 January 24, 2025 07:11 Inactive
@arslanashraf7 arslanashraf7 force-pushed the arslan/fix-external-sync branch from b49c0c4 to aef70d4 Compare January 24, 2025 07:11
@odlbot odlbot temporarily deployed to xpro-ci-pr-3385 January 24, 2025 07:12 Inactive
self.stdout.write(self.style.ERROR(f"Unknown vendor name {vendor_name}."))
return

platform = Platform.objects.filter(name__iexact=vendor_name).first()
if platform and not platform.enable_sync and not options.get("force"):
Copy link
Contributor Author

@arslanashraf7 arslanashraf7 Jan 24, 2025

Choose a reason for hiding this comment

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

The existence of the platform will be checked on line 38.

Copy link
Contributor

@Anas12091101 Anas12091101 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@arslanashraf7 arslanashraf7 merged commit 645b5b4 into master Jan 28, 2025
7 checks passed
@arslanashraf7 arslanashraf7 deleted the arslan/fix-external-sync branch January 28, 2025 08:18
@odlbot odlbot mentioned this pull request Jan 31, 2025
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants