search

DigitalCommons / mykomap

A web application for mapping initiatives in the Solidarity Economy
3 stars 0 forks source link

[DotCoop – Misc] Support the configuration of which tab is selected on map startup. #231

Open ColmDC opened 5 months ago

ColmDC commented 5 months ago

Please track against the clockify project [DotCoop - Misc]

Description

Support ability to configure a MM to specify what tab is selected on startup within the side panel

Acceptance Criteria

ColmDC commented 5 months ago

Might be worth addressing while fixing https://github.com/DigitalCommons/mykomap/issues/226

rogup commented 4 months ago

@wu-lee Please could you review my code for this? It's in https://github.com/DigitalCommons/mykomap/tree/231-default-sidebar-tab

I've tested locally the things in the acceptance criteria by checking out https://github.com/DigitalCommons/owned-by-oxford-project/tree/public/dev in /ext

To do those actual tests for DotCoop and ICA, do I just need to do the same thing but with those respective repos checked out in /ext? Or is there another way to test changes non-locally in some sort of staging setup?

Once the feature is ready to be applied, I'd also like to have a call about your process of rebasing the branch.

rogup commented 4 months ago

Also I've noticed that some of the line breaks have changed due to my Prettier code formatter.

Since more of us are going to be working on this, I'd be in favour of all of us using Prettier with the same default config, so that styling doesn't become inconsistent. This might cause a lot of changes for a short while, which makes looking at diffs a bit annoying, but I think it will be worth it in the long run - what do you think?

rogup commented 4 months ago

Oh, and should I be adding a unit test (or any other type of test) for this? If so, where should it go?

And are there any docs yet to describe our testing setup, and how we should be adding tests? I couldn't find any

wu-lee commented 4 months ago

To do those actual tests for DotCoop and ICA, do I just need to do the same thing but with those respective repos checked out in /ext? Or is there another way to test changes non-locally in some sort of staging setup?

Yes, that's how I do it. Although: I check out the map projects in directories adjacent to the mykomap working directory, and use a symlink. I think you're on OS X so this solution should work for you.

On Windows, for various reasons on top of this one, I'd recommend developing using Windows System for Linux. This isn't a commonly used platform for development, even then.

Once the feature is ready to be applied, I'd also like to have a call about your process of rebasing the branch.

Yep.

wu-lee commented 4 months ago

Also I've noticed that some of the line breaks have changed due to my Prettier code formatter.

Since more of us are going to be working on this, I'd be in favour of all of us using Prettier with the same default config, so that styling doesn't become inconsistent. This might cause a lot of changes for a short while, which makes looking at diffs a bit annoying, but I think it will be worth it in the long run - what do you think?

I'm not absolutely against this idea in theory, but - yikes! - I am baulking a bit at reformatting the entire project (and presumably all the other projects?) to support Prettier, which isn't something new to me and to all DCC development to date (i.e. not just me and not just this project). And especially doing that as part of this issue. (I'm not saying you're suggesting that, BTW, but it does seem to be a logical consequence if we did adopt it.)

I don't use VS Code for development, I use Emacs - so I'm wondering if I even can use Prettier. Investigating... yes, it's a stand-alone NodeJS tool, and it does seem to have support for Emacs, which is a relief, as otherwise it might be a step backwards for me.

Next, I was wondering how hard it would be to just tell Prettier to follow the current style in the repo.... (Which isn't totally consistent, but that's partly because of the chequered history, and my slight reluctance to make big whitespace changes like this unless I really have to.)

...However, reading the Prettier docs it says:

Prettier is not a kitchen-sink code formatter that attempts to print your code in any way you wish. It is opinionated. [...] By far the biggest reason for adopting Prettier is to stop all the ongoing debates over styles.

So ok: it it sounds like that's not an option. In fact that sounds a bit like Douglas Crockford's JS linter, which when I did try using, found unpleasant and inconvenient in practice exactly because if it's "opinionated and inflexible by design" design. It made me jump through hoops I didn't really feel were very helpful, and typically at times I didn't want to have to tell it to shut up and go away, and ultimately I concluded the simplest things was just to uninstall the little ****er.

So finally, I am wondering if this claim that our lives would be better with Prettier is actually valid.

But, even conceding that Prettier's vision that it adds productivity by silencing sources of dissent in a sane way to apply across the board on new projects... this would still imposes a great big multi-repo alteration which could scupper git-blame's ease of application from there on. Being able to use git-blame is itself a very useful tool for extracting information from the commit history. So I'm not yet convinced that vision offers a compelling benefit over using a formatter which can adopt a formatting style which matches, or closely matches, the one in the code already?

wu-lee commented 4 months ago

Oh, and should I be adding a unit test (or any other type of test) for this? If so, where should it go?

Tests would go in the test/ directory. They're all added by me late in the life of Mykomap, which wasn't designed to be test-first, and so are a bit piecemeal and best-effort. (And currently not all passing, I notice.)

Thing is, I'm not sure if I found a way to test the config very simply. test-model-config.js is an attempt. You can run that by itself with

npm run mocha-test test-model-config.js

Open to ideas.

And are there any docs yet to describe our testing setup, and how we should be adding tests? I couldn't find any

No, the testing that's here is a bit tentative, and added in a period when I was the only developer. I should write something.

ColmDC commented 4 months ago

But, even conceding that Prettier's vision that it adds productivity by silencing sources of dissent in a sane way to apply across the board on new projects... this would still imposes a great big multi-repo alteration which could scupper git-blame's ease of application from there on. Being able to use git-blame is itself a very useful tool for extracting information from the commit history. So I'm not yet convinced that vision offers a compelling benefit over using a formatter which can adopt a formatting style which matches, or closely matches, the one in the code already?

Can we move this discussion to element?

ColmDC commented 4 months ago

No, the testing that's here is a bit tentative, and added in a period when I was the only developer. I should write something.

Ditto.

wu-lee commented 4 months ago

(BTW I'm fine if you want to make this a pull-request - although it is essentially one commit ATM, it gives me somewhere to put the review comments.)

On a basic scan with "ignore whitespace" enabled, which removes some of the Prettier churn, it still a bit hard to find the bits which are not churn, because of all the quote-switching and line-rewrapping.

Note, this is all just from eyeballing - I've not yet had time to test this builds and does what it says, will defer that until tomorrow.

rogup commented 4 months ago

@wu-lee I've made a PR and addressed those comments https://github.com/DigitalCommons/mykomap/pull/236

ColmDC commented 4 months ago

Where can I test this feature? @rogup

rogup commented 4 months ago

@ColmDC what's the process for making features available for testing, and are there docs for this? I can add them if so

The way I tested it was to checkout 231-default-sidebar-tab and build/run it locally

ColmDC commented 4 months ago

@wu-lee might need to assist here. But from my perspective I need to be able to evaluate it on

https://dev.maps.coop/ica/

Demonstrate it working on ICA map with no config settings provided, so it should still default to Directory.

https://dev.maps.coop/dotcoop/

Demonstrate it working on DotCoop where the Search panel is visible on startup.

ColmDC commented 4 months ago

But if we want to move to alingning our processes, @lin-d-hop might prefer a different approach, like bundling a few more features before creating a new release?

wu-lee commented 3 months ago

What I used to do was to use hook-runner to deploy mykomap in "test mode" on https://dev.maps.coop/temp/

Currently there are hook-runner targets for testing Mykomap with the Co-ops UK map (config: coopsuk_testing.json) and the ObO map (config obo-public_testing.json). These are configured to deploy here:

The use of temp in the URL is meant to imply "don't rely on these working or even being here!"

What these targets do is redeploy the testing branch of the Mykomap repo in development mode, using the respective map project, and are triggered via webhooks from GitHub on commits to that branch.

To do the ICA would need adding a new hook-runner target which tests Mykomap using that map project. Alternatively you could add the option you're testing to one of the existing map projects (maybe ObO?) It'd need to be on the public/dev branch for the owned-by-oxford-project map.

And then you reset the testing branch of the Mykomap repo to be on the commit you want to test. Then when you push this branch to GitHub, it triggers a rebuild, which can be tested on the respective URL above. (The testing branch is intended for being yanked around like this, although with two of us we'd need to coordinate.)

Hope that makes sense! If not I can try and explain better.

wu-lee commented 3 months ago

Note, this is all done on dev-1, which can't use the dev.maps.coop as that's pointed at dev-2. When we migrate dev-1 properly the dev.maps.solidarityeconomy.coop domain will be deleted.

ColmDC commented 3 months ago

Doesn't seem to make any sense to start doing more on dev-1. Seems like we need do whatever @rogup needs to be able to deploy to dev-2 so I can test dev.maps.coop?

rogup commented 3 months ago

Yeah maybe it doesn't make sense to add new hookrunner targets for ICA and DotCoop to dev-1 if things are moving to dev-2.

I'm guessing adding hookrunner targets to dev-1 is less work than setting up the whole hookrunner on dev-2? But maybe we should bite the bullet and do it since we'll need this soon anyway.

I think @wu-lee is best placed to set up the hook runner on dev-2 since he made it and knows how it works for mykomap. Then I'm happy to sort out the testing branch to point at this fix.

@ColmDC up to you to decide how to prioritise this, so that the testing of this feature can be unblocked.

wu-lee commented 3 months ago

The only problem with putting things like this on dev-2 is that there's a bunch of stuff set up on dev-1 already. So it means either having two hook-runners running in parallel, which is confusing in various ways, or moving the whole lot, which is much more work and adds more cans of worms to something which is otherwise relatively simple. (All the git projects need reconfiguring manually to send their webhook notifications to a new URL, for example.)

Just adding this to the existing dev-1 instance would mean that it gets moved with everything else when that happens - so it won't get lost.

If we want to do this quick, I recommend using the hook-runner we have now.

The move to dev-2 can be done too, perhaps in parallel, but seems to be lower down the priority than various other tickets, including this one, and so this issue might get blocked by that.

rogup commented 2 months ago

@colm This is ready for QA.

I've set up the hook runner according to the Git workflow designed here by me and @wu-lee , for the ICA and DotCoop maps.

Since this feature was merged into dev, the QA builds were triggered and are available at https://dev.maps.solidarityeconomy.coop/qa/

Note that these are still on dev-1. It was too much work to move everything to dev-2 at this stage, since the hook runner isn't set up there, but once we have trialled this on dev-1 and setup some automation scripts, it should be easier to move it over to dev-2.

ColmDC commented 2 months ago

Demonstrate it working on DotCoop where the Search panel is visible on startup. For the full effect I realise defaultOpenSidebar also needs to be set to true which annoyingly doesn't seem to be settible by url, but the feature as spec'ed works.

Demonstrate it working on ICA map with no config settings provided, so it should still default to Directory. As above.

Happy to close this, and can someone set defaultOpenSidebar to be true for future dotcoop releases.

wu-lee commented 2 months ago

Happy to close this, and can someone set defaultOpenSidebar to be true for future dotcoop releases.

I've updated this on the dev branch and pushed - this has updated the deployed version here:

https://dev.maps.solidarityeconomy.coop/qa/dotcoop/

The version here used to be deployed by pushes to dev but I think has to be manually deployed with a push to staging now, is that right @rogup? (BTW your diagram link above goes to matrix.to?)

https://dev.dotcoop.solidarityeconomy.coop/

rogup commented 2 months ago

Here's the link to the editable diagram: https://drive.google.com/file/d/1Fu8zt5fdMXIXnyZz944CqwAo-vKdsFYY/view?usp=sharing

https://dev.dotcoop.solidarityeconomy.coop/ now needs to be manually deployed, by setting up the alpha branch on the dotcoop repo, and then clicking the dotcoop_alpha target on the hook runner. There are more details instructions on the diagram, which maybe we could eventually move to the Mykomap Wiki or docs

wu-lee commented 2 months ago

This has been tested - seems to be working, and is ready for merging to the main branch.