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

[wip] Backoffice UI Uplift #11007

Closed
wants to merge 0 commits into from
Closed

[wip] Backoffice UI Uplift #11007

wants to merge 0 commits into from

Conversation

dacook
Copy link
Member

@dacook dacook commented Jun 14, 2023

What? Why?

This is a shared development branch. We will likely rebase/rewrite history before submitting a PR to master.

Log of changes:

  1. DC: Add products_v3 page with simple table and styling
  2. JB: Add async loading of products table with spinner

What should we test?

  • Visit ... page.

Release notes

Changelog Category: 😎 Feature toggle

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

@dacook dacook force-pushed the backoffice-ui-uplift branch from b7c6fda to 9014e4a Compare June 14, 2023 06:48
@jibees jibees mentioned this pull request Jun 19, 2023
4 tasks
@jibees
Copy link
Contributor

jibees commented Jun 19, 2023

There are some merge commits that should probably be removed to clean git history...

I've rebased onto master, seems fine.

@jibees jibees force-pushed the backoffice-ui-uplift branch from 9446f1b to 7b9c42d Compare June 19, 2023 13:45
@mariocarabotta mariocarabotta added the pr-staged-au staging.openfoodnetwork.org.au label Jun 20, 2023
@jibees jibees added pr-staged-fr staging.coopcircuits.fr and removed pr-staged-fr staging.coopcircuits.fr labels Jun 20, 2023
@jibees jibees force-pushed the backoffice-ui-uplift branch from e676672 to 5d2b00a Compare June 21, 2023 10:08
@sigmundpetersen sigmundpetersen removed the pr-staged-au staging.openfoodnetwork.org.au label Jun 21, 2023
@dacook dacook added pr-staged-au staging.openfoodnetwork.org.au and removed pr-staged-au staging.openfoodnetwork.org.au labels Jun 22, 2023
@dacook dacook force-pushed the backoffice-ui-uplift branch from 927c34d to 09d9ebe Compare June 22, 2023 05:32
@jibees jibees force-pushed the backoffice-ui-uplift branch from 09d9ebe to d0f1cfe Compare June 22, 2023 09:04
@dacook dacook force-pushed the backoffice-ui-uplift branch from d0f1cfe to bb9036f Compare June 23, 2023 05:24
@jibees jibees force-pushed the backoffice-ui-uplift branch from bb9036f to 525e012 Compare June 26, 2023 09:37
@jibees jibees force-pushed the backoffice-ui-uplift branch from 612e911 to 1465320 Compare June 27, 2023 15:04
@dacook dacook force-pushed the backoffice-ui-uplift branch from 1465320 to 58bf29f Compare June 28, 2023 00:30
@dacook
Copy link
Member Author

dacook commented Jun 28, 2023

Rebased on top of #11123

@jibees jibees force-pushed the backoffice-ui-uplift branch 3 times, most recently from ba80c2d to 92ace71 Compare June 30, 2023 14:16
@jibees jibees force-pushed the backoffice-ui-uplift branch from c3ad687 to 5d03228 Compare July 3, 2023 13:39
@jibees
Copy link
Contributor

jibees commented Jul 25, 2023

@dacook I'd be happy if we could create PRs with commits from this PR in order to close this PR/branch, as I don't think it's still relevant

@jibees jibees force-pushed the backoffice-ui-uplift branch from 6dae2a5 to 7510a09 Compare July 25, 2023 14:01
@jibees
Copy link
Contributor

jibees commented Jul 25, 2023

I've created a PR with 4 commits that tweak the dropdown filters to make them compliant to v3 version: #11275

@jibees
Copy link
Contributor

jibees commented Jul 25, 2023

@dacook I'd propose you two create one (or more?) PR with the remaining commits of this PR in order to close it after that. Let me know what you think about it! 🙏

@jibees jibees force-pushed the backoffice-ui-uplift branch from 7510a09 to 2b8fa42 Compare July 25, 2023 15:53
@jibees
Copy link
Contributor

jibees commented Jul 25, 2023

@dacook
spec/system/admin/products_v3/products_spec.rb:46 is failing actually. don't know why ... maybe a bad conflict resolution?

@dacook dacook force-pushed the backoffice-ui-uplift branch from 2b8fa42 to cba11ce Compare July 26, 2023 02:23
@dacook
Copy link
Member Author

dacook commented Jul 26, 2023

This is really strange and had me stumped earlier. I think I've narrowed it down now, see latest commit. It seems due to an inconsistency between operating systems!!
Admittedly, I'm on an old macOS 10.15, and am overdue for an upgrade. It seems the behaviour might be fixed in later versions, so I'm wondering what version you use?

And secondly, unfortunately it seems the other system specs on this page seem quite flaky, I guess due to StimulusReflex..

@jibees

This comment was marked as resolved.

@jibees

This comment was marked as resolved.

@dacook
Copy link
Member Author

dacook commented Jul 26, 2023

Thanks for confirming. The spec actually passes in CI, so I guess Postgres on Macs behaves differently here, which is quite strange, I'm surprised we haven't encountered this before, I guess our sorting specs don't cover this case (eg this spec is just uppercase).

I think I'll just update the spec to avoid comparing lower/uppercase letters, and add a note on https://github.com/openfoodfoundation/openfoodnetwork/wiki/Development-Environment-Setup%3A-OS-X . Sound reasonable?

BTW A different spec did fail in the last build (see products_spec.rb:119), but I believe it's flaky.

@jibees
Copy link
Contributor

jibees commented Jul 26, 2023

I think I'll just update the spec to avoid comparing lower/uppercase letters, and add a note on https://github.com/openfoodfoundation/openfoodnetwork/wiki/Development-Environment-Setup%3A-OS-X . Sound reasonable?

Yes, I guess so.

@jibees
Copy link
Contributor

jibees commented Jul 26, 2023

BTW A different spec did fail in the last build (see products_spec.rb:119), but I believe it's flaky.

Pass locally. 👌

@dacook dacook force-pushed the backoffice-ui-uplift branch 2 times, most recently from ba372fc to f6a64f0 Compare July 27, 2023 01:21
@dacook dacook force-pushed the backoffice-ui-uplift branch 3 times, most recently from 868b648 to 73bfe63 Compare August 10, 2023 03:47
@jibees jibees force-pushed the backoffice-ui-uplift branch from 91f6701 to 16a00a8 Compare August 10, 2023 09:05
@jibees
Copy link
Contributor

jibees commented Aug 10, 2023

@dacook
Do you intend to merge this branch? I'm a bit confused, and at the same time worried about this branch that's always open.
Thanks for your insight.

@dacook
Copy link
Member Author

dacook commented Aug 11, 2023

Ugh, yes sorry I wasn't really thinking about that.

Maybe the process we need is to create PRs more often, and request a review from each other first.
If your commits are ready for review, perhaps we can submit this PR for review now, as it is?

@jibees jibees force-pushed the backoffice-ui-uplift branch 3 times, most recently from 5bbb2f8 to 90fdda1 Compare August 14, 2023 07:25
@jibees
Copy link
Contributor

jibees commented Aug 14, 2023

In order to finally close this one, I've opened two PRs: #11400, and #11401

After merging those two PRs, this one should not contains commits, and should therefore be closed.

@jibees jibees force-pushed the backoffice-ui-uplift branch from 90fdda1 to e9640d7 Compare August 14, 2023 12:42
@jibees jibees closed this Sep 5, 2023
@jibees jibees force-pushed the backoffice-ui-uplift branch from e9640d7 to a096c56 Compare September 5, 2023 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants