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.

Sign up for GitHub

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

Jump to bottom

Develop Issues fix PR #68

Merged
merged 10 commits into from
Jan 30, 2021
Merged

Develop Issues fix PR #68

merged 10 commits into from
Jan 30, 2021

Conversation

Ne-wt
Copy link
Contributor

@Ne-wt Ne-wt commented Jan 19, 2021

Fixed majority of issues (did not fix / attempt to fix issue #38)

All fixes where tested locally, logged in and out for those that could be affected by auth

preimpression
preimpression suggested changes Jan 19, 2021
View reviewed changes
Copy link
Contributor

@preimpression preimpression left a comment

Choose a reason for hiding this comment

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

A handful of suggestions. I only tested one thing, but I've seen the query->visible() bit used previously in something I believe Mercury did

app/Http/Controllers/Users/UserController.php Show resolved Hide resolved
composer.json Show resolved Hide resolved
resources/views/user/characters.blade.php Outdated Show resolved Hide resolved
resources/views/user/profile.blade.php Show resolved Hide resolved
app/Services/ShopService.php Outdated Show resolved Hide resolved
@preimpression
Copy link
Contributor

Looking good so far! I'd suggest also handling Sublists and MYOs the same way!
Also, the masterlist might need the same functionality added

@itinerare itinerare added the needs review Pull requests that are pending community review label Jan 23, 2021
itinerare
itinerare reviewed Jan 23, 2021
View reviewed changes
Copy link
Collaborator

@itinerare itinerare left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good by and large! Mostly curious about doctrine/dbal and if you plan to follow Uri's suggestion re ML/sublists/MYOs?

This was linked to issues Jan 23, 2021
MYO submission open/close site setting has no effect, uses design update site setting instead #23
Closed
Attempting to edit stock of a store with "free" (cost 0) items errors #26
Closed
Pending and draft design updates not cancelled / deleted on transfer #41
Closed
Hidden characters are not visible for those with manage_characters in a user's characters list. #47
Closed
Hidden Characters still referenced in ownership history #57
Closed
@Ne-wt
Copy link
Contributor Author

Ne-wt commented Jan 25, 2021

added to myos/sublists and also added a missing use Settings to gallery collaborator model that caused a 500 error when trying to comment

@itinerare itinerare merged commit cdea9d1 into corowne:develop Jan 30, 2021
@itinerare itinerare added this to the v2.0.0 milestone Jan 30, 2021
@itinerare itinerare added reviewed Pull requests that have received community review and are pending merge and removed needs review Pull requests that are pending community review labels Feb 17, 2021
CH3RVB pushed a commit to CH3RVB/lorekeeper that referenced this pull request Jan 29, 2024
@Ne-wt
Merge pull request corowne#68 from itinerare/extension/sellable-charas
b14186f 
Some bugfixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@preimpression preimpression preimpression requested changes

@Draginraptor Draginraptor Draginraptor approved these changes

@itinerare itinerare itinerare approved these changes

Assignees
No one assigned
Labels
reviewed Pull requests that have received community review and are pending merge
Projects
None yet
Milestone
v2.0.0
4 participants
@Ne-wt @preimpression @Draginraptor @itinerare