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

Modified scope handling. #150

Merged
merged 2 commits into from Sep 30, 2022
Merged

Modified scope handling. #150

merged 2 commits into from Sep 30, 2022

Conversation

ghost
Copy link

@ghost ghost commented May 22, 2019 •
edited by ghost
Loading

Closes #99.

Moves reserved scopes definition out of the local config xml and into the scopes table in the database as a 'reserved' boolean associated with each scope entry. Addition of consequent changes to the interface to allow those with admin privileges to set/unset the reserved state flag on a scope at creation and if editting.

Would prefer not to use '!important' in web_portal.css which serves to left-align the 'reserved' checkbox correctly under the text entry boxes for name and description in the add/edit scope dialogues.

@gregcorbett gregcorbett self-assigned this May 29, 2019
@gregcorbett
Copy link
Member

Maybe we can look at the effect of having and not having !important in web_portal.css. at our next meeting? Or email around some screenshots highlight the difference?

@gregcorbett gregcorbett added this to the 5.8.0 milestone May 29, 2019
@tofu-rocketry
Copy link
Member

Special rules like that seem a generally bad idea - Why was !important required here?

@tofu-rocketry
Copy link
Member

We shall ignore the Codacy issue as get is consistent with existing object model.

jrha
jrha previously approved these changes May 30, 2019
View reviewed changes
Copy link
Contributor

@jrha jrha left a comment

Choose a reason for hiding this comment

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

As discussed we decided that all in all !important wasn't that bad in the scheme of things.

tofu-rocketry
tofu-rocketry requested changes May 30, 2019
View reviewed changes
Copy link
Member

@tofu-rocketry tofu-rocketry left a comment

Choose a reason for hiding this comment

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

I feel like these changes could do with more documentation on them, either in the commit message or PR.

tofu-rocketry
tofu-rocketry previously approved these changes Jun 12, 2019
View reviewed changes
@gregcorbett
Copy link
Member

As discussed yesterday, @ineilson please do add a second commit that removes the now unused code for reading the reserved scopes from a local config and changes any relevant comments.

@gregcorbett
Copy link
Member

gregcorbett commented Jun 13, 2019 •
edited
Loading

I have also "ignored" the codacy issue: https://app.codacy.com/app/gocdb/gocdb/issues/ignored because #wontfix

@tofu-rocketry tofu-rocketry self-requested a review June 13, 2019 09:33
@tofu-rocketry tofu-rocketry dismissed their stale review June 13, 2019 09:33

Changes needed

@ghost ghost dismissed jrha’s stale review via c85e02f June 13, 2019 15:08
tofu-rocketry
tofu-rocketry previously approved these changes Jun 13, 2019
View reviewed changes
@gregcorbett gregcorbett self-requested a review June 14, 2019 07:43
gregcorbett
gregcorbett previously approved these changes Jun 14, 2019
View reviewed changes
Copy link
Member

@gregcorbett gregcorbett left a comment

Choose a reason for hiding this comment

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

Thanks Ian, I've added this PR to the 5.8.0 milestone, so will leave it un-merged till then.

jrha
jrha previously approved these changes Jun 17, 2019
View reviewed changes
@ghost ghost dismissed stale reviews from jrha, gregcorbett, and tofu-rocketry via 8fe5389 September 4, 2019 13:37
@tofu-rocketry tofu-rocketry self-requested a review September 4, 2019 13:50
tofu-rocketry
tofu-rocketry previously approved these changes Sep 4, 2019
View reviewed changes
@ghost
Copy link
Author

ghost commented Sep 9, 2019

Finger (or brain) trouble, accidentally force pushed outdated scopes branch. Apologies.

@gregcorbett gregcorbett added the schema change Issues or PRs that require a schema change to implement. label May 27, 2021
@gregcorbett gregcorbett removed this from the 5.8.0 milestone May 27, 2021
@gregcorbett gregcorbett added this to the 5.10.0 milestone May 27, 2021
@gregcorbett gregcorbett modified the milestones: 5.10.0, August 2022 Jul 11, 2022
@gregcorbett gregcorbett modified the milestones: August 2022, November 2022 Sep 6, 2022
Ian Neilson added 2 commits September 30, 2022 11:22
@gregcorbett
Basic scope creation and tag edits functional with tag display added to
105e935 
edit/success screens
@gregcorbett
Remove unused code and adjust comments
bbddb16 
Removed unused method for loading reserved scopes from local_config.xml and adjust some comments.
@gregcorbett gregcorbett requested a review from a team as a code owner September 30, 2022 11:39
@gregcorbett
Copy link
Member

I've rebased this and checked nothing got lost or broken during the process.

@gregcorbett gregcorbett merged commit 74fe830 into GOCDB:dev Sep 30, 2022
@gregcorbett gregcorbett modified the milestones: November 2022, June 2023 Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GRyall GRyall Awaiting requested review from GRyall

Assignees

@gregcorbett gregcorbett

Labels
enhancement schema change Issues or PRs that require a schema change to implement.
Projects
None yet
Milestone
5.11.0
Development

Successfully merging this pull request may close these issues.

Improve handling of reserved scopes
3 participants
@gregcorbett @tofu-rocketry @jrha