Skip to content
This repository has been archived by the owner on Aug 21, 2024. It is now read-only.

Changes for location services to use schema, knex & feathers 5 #8103

Merged
merged 55 commits into from
Jul 27, 2023

Conversation

hanzlamateen
Copy link
Member

@hanzlamateen hanzlamateen commented Jun 20, 2023

Summary

This PR Covers following services migration to feathers 5:

  • location
  • location-type
  • location-setting

References

#7645, closes #8284

Checklist

  • If this PR is still a WIP, convert to a draft
  • When this PR is ready, mark it as "Ready for review"
  • ensure all checks pass
  • Changes have been manually QA'd
  • Changes reviewed by at least 2 approved reviewer

QA Steps

List any additional steps required to QA the changes of this PR, as well as any supplemental images or videos.

@hanzlamateen hanzlamateen marked this pull request as ready for review July 25, 2023 17:44
Copy link
Member

@barankyle barankyle left a comment

Choose a reason for hiding this comment

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

I've noticed that selecting a location type when creating/editing a location in /admin/locations does not work ATM; no options are populating the selector, so it cannot be used. Since I see this on dev as well, it seems this PR didn't break it, but please fix it since it fits with everything else this PR is doing.

I also noticed that if a user goes to a location they are not authorized for, the Not Authorized notification is not shown. From the logs it looks like that action it getting dispatched, but it's not being picked up, or something. This can be tested by making a private location, then opening a new browser profile and trying to go to that location as a guest.

@hanzlamateen
Copy link
Member Author

hanzlamateen commented Jul 26, 2023

I've noticed that selecting a location type when creating/editing a location in /admin/locations does not work ATM; no options are populating the selector, so it cannot be used. Since I see this on dev as well, it seems this PR didn't break it, but please fix it since it fits with everything else this PR is doing.

I also noticed that if a user goes to a location they are not authorized for, the Not Authorized notification is not shown. From the logs it looks like that action it getting dispatched, but it's not being picked up, or something. This can be tested by making a private location, then opening a new browser profile and trying to go to that location as a guest.

I have fixed the location type not appearing in location drawer issue.

Though for Not Authorized notification, I am not sure why its not appearing. I looked into the code and its been called properly. It seems like WarningUISystem is having some problem internally. Its not showing up the ui despite the WarningUIService.openWarning being called for above scenario. Its just something in xrui and partically around following line call which is causing it not to appear. Maybe @HexaField @speigg might have a better idea.

setComponent(ui.entity, ComputedTransformComponent, {

@HexaField
Copy link
Member

I've noticed that selecting a location type when creating/editing a location in /admin/locations does not work ATM; no options are populating the selector, so it cannot be used. Since I see this on dev as well, it seems this PR didn't break it, but please fix it since it fits with everything else this PR is doing.

I also noticed that if a user goes to a location they are not authorized for, the Not Authorized notification is not shown. From the logs it looks like that action it getting dispatched, but it's not being picked up, or something. This can be tested by making a private location, then opening a new browser profile and trying to go to that location as a guest.

I have fixed the location type not appearing in location drawer issue.

Though for Not Authorized notification, I am not sure why its not appearing. I looked into the code and its been called properly. It seems like WarningUISystem is having some problem internally. Its not showing up the ui despite the WarningUIService.openWarning being called for above scenario. Its just something in xrui and partically around call

setComponent(ui.entity, ComputedTransformComponent, {
which is causing it not to appear. Maybe @HexaField @speigg might have a better idea.

Will look into this

@hanzlamateen hanzlamateen added this pull request to the merge queue Jul 27, 2023
Merged via the queue into dev with commit b1a7408 Jul 27, 2023
8 checks passed
@hanzlamateen hanzlamateen deleted the location-feathers5 branch July 27, 2023 05:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate location services to feathers 5
3 participants