Skip to content
This repository has been archived by the owner on Jun 9, 2021. It is now read-only.

Button not displaying on PRs which are merged #197

Open
itay opened this issue Feb 14, 2017 · 45 comments
Open

Button not displaying on PRs which are merged #197

itay opened this issue Feb 14, 2017 · 45 comments

Comments

@itay
Copy link
Contributor

itay commented Feb 14, 2017

We have a trigger that is configured like this:

image

I can see the API returning it:
image

However, it's not being rendered:
image

Any idea what might be happening?

@tomasbjerre
Copy link
Owner

Which version of Bitbucket Server is that?

@itay
Copy link
Contributor Author

itay commented Feb 14, 2017

4.11.1

@tomasbjerre
Copy link
Owner

No errors in the console tab of developer tools?
Do you have other buttons that are working with 4.11.1?

@tomasbjerre
Copy link
Owner

Now I see it!

I just released 2.54 that changes the "state" label to "Ignore if". Which is what those 2 checkboxes does. They ignore the notification if the PR is merged/declined.

@ahadiwijaya
Copy link

ahadiwijaya commented Feb 16, 2017

@tomasbjerre Was the fix just the label? I tried changing the configuration by unchecking the "State" (="Ignore if") conditions, and the issue still persists - the button is not appearing for merged PRs.
screen shot 2017-02-16 at 12 06 46 pm

The button is not appearing for PRs that have been merged:
screen shot 2017-02-16 at 12 05 28 pm

And I can also see the API returning the button data:
screen shot 2017-02-16 at 12 10 11 pm

However, it is appearing for non-merged PRs:
screen shot 2017-02-16 at 12 08 25 pm

@tomasbjerre
Copy link
Owner

Yes that was the only change.

Perhaps the problem is a conflict with the Webhook to Jenkins plugin. If you disable that plugin, does the button from this plugin show up correctly also for unmerged PR:s?

@ahadiwijaya
Copy link

@tomasbjerre I believe the Webhook to Jenkins plugin has always been disabled for the repository - do you know what else could affect this?

@tomasbjerre
Copy link
Owner

The "Trigger Notification" -button in your first screenshot, is that not from the Webhook to Jenkins plugin?

@ahadiwijaya
Copy link

ahadiwijaya commented Feb 16, 2017

I think it's from PR notifier (?) Please correct me if I'm wrong.

WebFragments.addWebItemDescriptor({"completeModuleKey":"se.bjurr.prnfs.pull-request-notifier-for-stash:se.bjurr.prnfb.triggerbutton","key":"se.bjurr.prnfb.triggerbutton","moduleKey":"se.bjurr.prnfb.triggerbutton","params":{},"pluginKey":"se.bjurr.prnfs.pull-request-notifier-for-stash","location":"bitbucket.internal.pullrequest.toolbar.deprecated/bitbucket.pull-request.toolbar.actions","weight":50,"linkText":"Trigger Notification","styleClass":"triggerManualNotification"});

This fragment appears both on the merged and non-merged PRs, but I only see it as a button on merged PRs.

@tomasbjerre tomasbjerre reopened this Feb 16, 2017
@tomasbjerre
Copy link
Owner

Ok I will probably have some time to look at this tomorrow. Thanks for the info!

@ahadiwijaya
Copy link

Thanks @tomasbjerre !

@tomasbjerre
Copy link
Owner

I just tried it with Webhook plugin and there is no conflict. It works for me.

And you have no errors in the console log?
There are no invisible or space chars in your filter regexp or text?

@tomasbjerre
Copy link
Owner

I have configuration like this:

curl -u admin:admin 'http://localhost:7990/bitbucket/rest/prnfb-admin/1.0/settings/notifications' -H 'Content-Type: application/json; charset=UTF-8' -H 'Accept: application/json, text/javascript, */*; q=0.01'

Gives:

[{"filterRegexp":"Build AMI","filterString":"${BUTTON_TRIGGER_TITLE}","headers":[],"method":"GET","name":"Notification","triggerIfCanMerge":"NOT_CONFLICTING","triggerIgnoreStateList":[],"triggers":["BUTTON_TRIGGER"],"url":"http://localhost:7990/bitbucket/plugins/servlet/prnfb/admin","uuid":"0ba1a2fb-c6e9-48c0-848e-72bde5597402","postContentEncoding":"NONE"},{"headers":[],"method":"GET","name":"Notification","triggerIfCanMerge":"ALWAYS","triggerIgnoreStateList":[],"triggers":["RESCOPED_FROM","RESCOPED_TO"],"url":"http://localhost:80/?abc","uuid":"b1306a3a-5a87-4145-80b7-660bc986dd25","postContentEncoding":"NONE"}]
curl -u admin:admin 'http://localhost:7990/bitbucket/rest/prnfb-admin/1.0/settings/buttons/repository/1/pullrequest/1' -H 'Content-Type: application/json; charset=UTF-8' -H 'Accept: application/json, text/javascript, */*; q=0.01'

Gives:

[{"buttonFormList":[],"buttonFormListString":"[]","name":"Build AMI","uuid":"54fa6576-1dd2-4443-989e-bdb6ca3a238c"}]

And that button is also shown in GUI. What do you get with these curl commands?

@tomasbjerre
Copy link
Owner

I read this issue a bit to quick. It must be a frontend issue. Very hard to investigate unless there is an error in the console view of Developer Tools. Any other plugins you have installed that might cause it?

@tomasbjerre
Copy link
Owner

The "Trigger Notification" is added here:
https://github.com/tomasbjerre/pull-request-notifier-for-bitbucket/blob/master/src/main/resources/atlassian-plugin.xml#L59

It is just a placeholder that should be removed immediately when the page loads:
https://github.com/tomasbjerre/pull-request-notifier-for-bitbucket/blob/master/src/main/resources/pr-triggerbutton.js#L208

If is not removed, I would suspect that:

  • The javascript was never loaded, do you see in the resources tab in Developer Tools?
  • Or that it crashed, that should be clear from the console tab in Developer Tools.

@ahadiwijaya
Copy link

Hi,

On both cases, I see:
screen shot 2017-02-21 at 10 32 05 am

On the error console, I do not see any error corresponding to pr-triggerbutton resources. Does this imply that the function that executes the generation of the buttons on the UI was never called?

@tomasbjerre
Copy link
Owner

Strange... Do you have anyone there who knows some Javascript and can debug it? the pr-triggerbutton.js is not minified and can be debugged.

It should find a button, the one called "Trigger Button", remove it and add new buttons in its place.

@ahadiwijaya
Copy link

:'( Sure, I'll keep you updated once I find someone and find out what happened. Thanks!

@itay
Copy link
Contributor Author

itay commented Feb 21, 2017

@tomasbjerre I was trying to debug it, but Bitbucket minifies it automatically so it's basically impossible to debug on our production instance.

@tomasbjerre
Copy link
Owner

tomasbjerre commented Feb 21, 2017

Thanks for trying @itay =)

Maby try -Datlassian.webresource.disable.minification=true

https://developer.atlassian.com/docs/advanced-topics/supporting-minification-of-javascript-and-css-resources

@tomasbjerre
Copy link
Owner

Also, perhaps you can add the un-minified version from your local filesystem:

https://developers.google.com/web/tools/setup/setup-workflow

@itay
Copy link
Contributor Author

itay commented Feb 22, 2017

Unfortunately this is production system with over 1000 users - we can't mess with it :)

@tomasbjerre
Copy link
Owner

But if you add local pr-buttontrigger.js it will only affect you.

@itay
Copy link
Contributor Author

itay commented Feb 22, 2017

Ah, I misread it. We can give it a shot.

@tomasbjerre
Copy link
Owner

@aepfli Are your buttons returned from the rest-service?
Did you also install/update any other plugin when moving from 2.55 to 2.57?

@aepfli
Copy link

aepfli commented Mar 6, 2017

@tomasbjerre besides the workzone plugin - this was the only plugin we updated!

buttons REST CALL
[ { "buttonFormList": [], "buttonFormListString": "[]", "name": "trigger FeatureBuild", "projectKey": "****", "repositorySlug": "****", "userLevel": "EVERYONE", "uuid": "1f3d6cf3-fd62-4335-b28d-24b70471d5cb", "confirmationText": "Build has been Triggered! dont hit the button twice" } ]

@aepfli
Copy link

aepfli commented Mar 6, 2017

imho: is this "version" related... if so, maybe deactivate the version for others... so they wont get that problem!

@tomasbjerre
Copy link
Owner

If I understand correctly. @itay had the problem in 2.53. @aepfli had it working in 2.55.

But that is something you can do to help. Try older versions and try to identify one introducing the problem.

@aepfli
Copy link

aepfli commented Mar 6, 2017

i am right now getting a frontend dev.. to take a look with me... but, we also just use the minified version, as we cant switch for 500 users atm... also i will issue a restart of stash at night... maybe the old "quick fix" will help :D

but i will come back to this topic!

@tomasbjerre
Copy link
Owner

Great! Thanks :)

@aepfli
Copy link

aepfli commented Mar 6, 2017

okay so right now i am debugging this with the minified version, and chrome is helping a lot by pretty print...

strangely on page load there is no object ".triggerManualNotification" the array length is null... this is happening on the first time it steps into your js part...!

i hope this some information helps a little...!
2017-03-06_13h21_11

@aepfli
Copy link

aepfli commented Mar 6, 2017

as i am not really familiar with plugin development, i found something which can be related!

i saw you changed the "definition" of the item, from web-item to client-web-item in d1ca3cd

maybe those are still two different issues, and mine is related to my version 4.6.1 and the bug, which is explained in https://jira.atlassian.com/browse/BSERV-8896!

just some ideas! if i am on the wrong path, let me know!

@aepfli
Copy link

aepfli commented Mar 6, 2017

i read your code, and i am pretty aware of when and where stuff gets loaded.

but when i use jquery to get an object with class "triggerManualNotification" during debugging, on line 12, i get an empty array! So this object is not there, at that moment!, and isnt for the whole time, on every breakpoint, i cant find this element! So there is at least the bug for my problem! that this i not in the html!

And as i am using an "older" version -> this may relates to my comment 5 minutes ago! and my findings

@tomasbjerre
Copy link
Owner

OK! Sounds very interesting. Thanks. I will have a closer look at this later today.

For the record, that triggerManualNotification class should have been added from this config:
https://github.com/tomasbjerre/pull-request-notifier-for-bitbucket/blob/master/src/main/resources/atlassian-plugin.xml#L60

@aepfli
Copy link

aepfli commented Mar 6, 2017

yeah and in this file you should add:
web-resource key="fix-dependency-bug"> <context>bitbucket.page.pullRequest.view</context> <dependency>com.atlassian.bitbucket.server.bitbucket-client-web-fragments:pull-request-header-more-location</dependency> </web-resource>

well i checked your project out, and did the change already (not tested) but i cant push to your project... not familiar with github so far. but will dig into it, and try to provide a pull request!

tomasbjerre added a commit that referenced this issue Mar 6, 2017
 * Buttons are not showing. Probably because of an Atlassian Bug BSERV-8896.
tomasbjerre added a commit that referenced this issue Mar 6, 2017
 * Buttons are not showing. Probably because of an Atlassian Bug BSERV-8896.
@tomasbjerre
Copy link
Owner

I released this in 2.58 now. Really hope it works, thanks a lot @aepfli !

Please confirm if it works for you!

@aepfli
Copy link

aepfli commented Mar 7, 2017

Hi! Sadly 2.58 did not work! But we downgraded to version 2.54 and everything is running smoothly again!

I will get a docker with bitbucket running today, and evaluate different versions of bitbucket and 2.58! At least I will try, can't promise anything :)

tomasbjerre added a commit that referenced this issue Mar 7, 2017
 * This fix is not relying on the button being added by web-client-item configuration in atlassian-plugin.xml. The JS will now find the button-area without using the triggerManualNotification-class.
@tomasbjerre
Copy link
Owner

You may try 2.59.

@aepfli You do get the buttons from the REST resource tight?
http://localhost:7990/bitbucket/rest/prnfb-admin/1.0/settings/buttons/repository/1/pullrequest/1

@tomasbjerre
Copy link
Owner

Actually, 2.60!

@tomasbjerre
Copy link
Owner

@aepfli @ahadiwijaya @itay did any of you try this? Is it solved?

@aepfli
Copy link

aepfli commented Mar 10, 2017

Tried today just for 5 min, did not work... But so far I had not time to debug...

@tomasbjerre
Copy link
Owner

Manually pasting this line into the console and see what it evaluates to, would be interesting.

@aepfli
Copy link

aepfli commented Mar 10, 2017

Well right now I am 10 days out of office helping a junior team somewhere... So I will check after that!

@itay
Copy link
Contributor Author

itay commented Mar 10, 2017

I also happen to be on holiday this week and next :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants