Sync stable to main

Support Discourse Custom Wizard
, ,
1

We should be careful that stable will not run behind main more than necessary.

It seems like we never synced stable to main when Discourse 3.0 was released (which is a typical moment to sync them, since Discourse core is also synced then (Discourse 3.0 is 100% equal to Discourse 3.1.0 beta 1).

We should test whether the current main branch passes all tests on Discourse 3.0.

If so, we should make stable point to the current commit on main and keep it synchronized until we have an actual new feature that is not compatible with Discourse 3.0.

If not, we should see if there are any large issues and fix them if the effort is relatively low

2

Discourse 3.1 has been released so this is the perfect moment to perform this

we should make stable point to the current commit on main and keep it synchronized until we have an actual new feature that is not compatible with Discourse 3.2.0 beta-something.

This is also interesting.

Assigning to the maintainer since this is important but not very urgent (although it would be nice to do this somewhere in the upcoming week and treat it as top-of-the-stack). Maintainer can then decide who will perform the task.

1 Like
6

@jumagura Could you have a look at this one? We can discuss it in our next meeting.

1 Like
7

When is the next meeting? This should be only 5 minutes of work?

2 Likes
8

Hi Richard, Angus

I’ve created a PR for merging main into stable (link). While all tests passed locally on Discourse stable, a frontend test is failing in the GitHub CI environment.

The error is related to a modal change I made 5 months ago:
Error: QUnit Test Failure: Global error: Uncaught Error: Assertion Failed: Could not find "modal.insert-hyperlink" template, view, or component. at http://localhost:7357/assets/vendor.js, line 43396

I’m unable to identify the cause, as the assertion in the error doesn’t match what I’ve done.

Here are the relevant changes:

  • Addition of the modal: link
  • Acceptance test: link

Could you please help me understand what might be going wrong? My apologies if I’m overlooking something simple.

Thank you!

2 Likes
9

I think the test is failing because the code needs updating (so the test is working, i.e. it’s showing us there’s a problem). The issue is probably also present in main. It looks like we’re still using the old modal api in the custom wizard.

showModal

discourse/discourse now uses

this.modal.show

Let me know if you run into issues when addressing this in main and stable.

Thanks!

2 Likes
10

Yeah, this was a change recently.

On TLP I had to change a modal like so, you may need to do something similar:

Note this required a compatibility pin but if you are running two branches check if the change is also necessary on stable.

1 Like
11

Any idea why PMS doesn’t find that error then? :thinking:

1 Like
12

I’ll let Marcos have a shot at answering that. I don’t know if I’m right yet.

2 Likes
13

Hi everyone,

I’m reaching out to provide an update on the modal component issue I’ve been working on. Here’s a detailed breakdown of the situation:

  1. Attempt to Use the Template: My initial approach involved utilizing the existing Discourse modal template insert-hyperlink. However, this is where I encountered issues with the CLI tests, even though the functionality appears to be working.

  2. Suspecting the Need to Create a Component: I suspect that I may need to create a dedicated component instead of relying on the existing Discourse template. The new modal system in Discourse seems to require components, and the template alone may not be sufficient.

  3. Funny Twist: In an interesting turn of events, I had previously deleted a component that performed that function (apologies to @Angus) and replaced it with the template. It seems that I’ll need to restore that component or create a new one to align with Discourse’s updated modal system.

It’s a bit of a learning curve, and I’m still exploring the best approach. If anyone has faced similar challenges or has insights into working with the new modal system in Discourse, I’d appreciate any guidance or feedback.

Thanks for your continued support, and I look forward to any insights you may have!

2 Likes
14

It sounds like you’re on the right track Marcos. Keep going. You’ll get there.

15

I was missing a , that was the problem. I should have read Robert code better :sweat_smile: I fixed that deprecation problem, now all test are passing.

I created a PR to main and from there it would be merged to stable.

4 Likes
16

Reviewed, ran tests locally, manually tested as well and merged with thanks! :+1:

Does this need to be on stable too?

btw, one observation: the admin FE tests run quite long - do we know why that is the case? There seem to be a lot of long pauses?

1 Like
17

There are still a lot of differences between stable and main, also when I do diff -r between them.

image
image922×185 11.1 KB

It might be best to remove the stable branch and create a new, pristine, one.

1 Like
18

I can clarify that situation. When data is created, modified, or deleted in the admin tab(custom fields and manager tabs), a banner displays a message that returns to normal after 10 seconds. The test asserts this behavior in those six actions.

I initially tried to assert only the first change or even ignore it, but this led to a global failure. The error occurred after this test passed and other tests started running. It was caused when the message tried to return to its default state, and no object was found since another test was running.

I resolved the issue by asserting both the change and the return to normal, ensuring that the test functions correctly without errors.

image

image
image745×98 8.23 KB

We could do that. I agree with that. That would reduce to zero any merge problems :sweat_smile:

1 Like
19

Interesting.

is there anyway we can shorten that 10 seconds do you think?

20

Hi @development_team

Following @richard 's idea to synchronize the stable branch with main, here are the steps we can take:

  1. Reset the stable branch to the main branch.
  2. Force-push the stable branch to the remote repository.

Optionally, we can create a backup of the current stable branch to preserve its history.

This approach will align stable with main without losing the history of changes.

@angus, do you agree with this approach? If so, we can proceed with these steps. If you have any concerns or alternative suggestions, please let me know.

1 Like
21

I understand the need for efficiency. I’ll look into options to reduce this time. Will keep you updated.

1 Like
22

What about a very simple solution ?

Rename current stable to stable_old

Then branch current state of main to a new stable?

@richard would that work for you?

1 Like
23

That works - but only once so let’s call it stable_30 then.