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

Don't default to permanent if falsy permitType #320

Closed
wants to merge 1 commit into from

Conversation

ChinemeremChigbo
Copy link
Member

@ChinemeremChigbo ChinemeremChigbo commented Oct 3, 2023

The permit type field should never be falsy, but at times javascript eludes understanding

Notion ticket link

[16] For new temporary permit holders, under their Past APPs section, it lists them as having a permanent permit despite their recent permit request being a temporary one

Implementation description

  • I have still been unable to replicate this bug on my end, however I think it may be worth asking rcd if this change has any effect on their end.

Checklist

  • My PR name is descriptive, is in imperative tense and starts with one of the following: [Feature],[Improvement] or [Fix],
  • I have run the appropriate linter(s)
  • I have requested a review from the RCD team on GitHub, or specific people who are associated with this ticket

The permit type field should never be falsy, but at times javascript eludes understanding
@ChinemeremChigbo ChinemeremChigbo requested review from OustanDing, sherryhli and leogjhuang and removed request for OustanDing October 3, 2023 11:52
@sherryhli
Copy link
Member

but at times JavaScript eludes understanding

😂

@ChinemeremChigbo one thought, I think this may be related to #273, another of the unreleased fixes. If permit type was previously not queried from the backend, then it would be defaulting to PERMANENT on the frontend

@ChinemeremChigbo
Copy link
Member Author

but at times JavaScript eludes understanding

😂

@ChinemeremChigbo one thought, I think this may be related to #273, another of the unreleased fixes. If permit type was previously not queried from the backend, then it would be defaulting to PERMANENT on the frontend

Ahh interesting, should I close this PR then? I don't think that this permitType should ever be falsy, so I don't really think this change will make any difference.
image

@sherryhli
Copy link
Member

@ChinemeremChigbo Yeah let's close it. It seems #273 was indeed the fix based on RCD's latest email.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants